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

[Junio C Hamano <gitster@pobox.com>]

Dan Aloni <alonid@gmail.com> 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.