Re: [PATCH v4 3/3] ident.c: cleanup wrt ident's source

Dan Aloni [off-list ref] writes:

+static int ident_source_is_sufficient(enum ident_source source)
+{
+	switch (source) {
+	case IDENT_SOURCE_CONFIG:
+	case IDENT_SOURCE_ENVIRONMENT:
+		return 1;

Without adding these two lines here:

	default:
        	break;

I get this build failure:

ident.c: In function 'ident_source_is_sufficient':
ident.c:444:2: error: enumeration value 'IDENT_SOURCE_UNKNOWN' not handled in switch [-Werror=switch]
  switch (source) {
  ^
ident.c:444:2: error: enumeration value 'IDENT_SOURCE_GUESSED' not handled in switch [-Werror=switch]
ident.c:444:2: error: enumeration value 'IDENT_SOURCE_GUESSED_BOGUS' not handled in switch [-Werror=switch]

+static int ident_is_sufficient(enum ident_person person)
 {
+	const char *str_name, *str_email;
+	int name, email;
+
+	switch (person) {
+	case IDENT_PERSON_COMMITTER:
+		str_name = getenv("GIT_COMMITTER_NAME");
+		str_email = getenv("GIT_COMMITTER_EMAIL");
+		break;
+	case IDENT_PERSON_AUTHOR:
+		str_name = getenv("GIT_AUTHOR_NAME");
+		str_email = getenv("GIT_AUTHOR_EMAIL");
+		break;
+	default:
+		die("invalid parameter to ident_is_sufficient()");
+	}
+
+	name = !!str_name || ident_source_is_sufficient(source_of_default_name);
+	email = !!str_email || ident_source_is_sufficient(source_of_default_email);
+
 #ifndef WINDOWS
-	return (user_ident_explicitly_given & IDENT_MAIL_GIVEN);
+	return email;
 #else
-	return (user_ident_explicitly_given == IDENT_ALL_GIVEN);
+	return name && email;
 #endif
 }

It appears that str_name and name are unconditionally computed even
though it does not affect the outcome of the whole thing.  When
building for !WINDOWS, I get

ident.c: In function 'ident_is_sufficient':
ident.c:456:6: error: variable 'name' set but not used [-Werror=unused-but-set-variable]
  int name, email;
      ^
cc1: all warnings being treated as errors
make: *** [ident.o] Error 1

as the result of this.

On Thu, Feb 04, 2016 at 02:42:55PM -0800, Junio C Hamano wrote:

Dan Aloni [off-list ref] writes:

quoted

+static int ident_source_is_sufficient(enum ident_source source)
+{
+	switch (source) {
+	case IDENT_SOURCE_CONFIG:
+	case IDENT_SOURCE_ENVIRONMENT:
+		return 1;

Without adding these two lines here:

	default:
        	break;

I get this build failure:

ident.c: In function 'ident_source_is_sufficient':
ident.c:444:2: error: enumeration value 'IDENT_SOURCE_UNKNOWN' not handled in switch [-Werror=switch]
  switch (source) {
  ^
ident.c:444:2: error: enumeration value 'IDENT_SOURCE_GUESSED' not handled in switch [-Werror=switch]
ident.c:444:2: error: enumeration value 'IDENT_SOURCE_GUESSED_BOGUS' not handled in switch [-Werror=switch]

My fault - must have left over CFLAGS="-O0 -g" while debugging, so it
got missed. Will fix.

[..]
ident.c: In function 'ident_is_sufficient':
ident.c:456:6: error: variable 'name' set but not used [-Werror=unused-but-set-variable]
  int name, email;
      ^
cc1: all warnings being treated as errors
make: *** [ident.o] Error 1

Ditto.

-- 
Dan Aloni