From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 21 Mar 2013 19:55:12 GMT
Subject: Re: [PATCH 1/4] wt-status: fix possible use of uninitialized variable
Message-ID: <7vip4ka5pb.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130321194949.GG29311@google.com>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Jeff King wrote:
>
>> Instead of using the "x = x" hack, let's handle the default
>> case in the switch() statement with a die("BUG"). That tells
>> the compiler and any readers of the code exactly what the
>> function's input assumptions are.
>
> Sounds reasonable.
>
>> We could also convert the flag to an enum, which would
>> provide a compile-time check on the function input.
>
> Unfortunately C permits out-of-bounds values for enums.
>
> [...]
>> --- a/wt-status.c
>> +++ b/wt-status.c
>> @@ -264,7 +264,7 @@ static void wt_status_print_change_data(struct wt_status *s,
>>  {
>>  	struct wt_status_change_data *d = it->util;
>>  	const char *c = color(change_type, s);
>> -	int status = status;
>> +	int status;
>>  	char *one_name;
>>  	char *two_name;
>>  	const char *one, *two;
>> @@ -292,6 +292,9 @@ static void wt_status_print_change_data(struct wt_status *s,
>>  		}
>>  		status = d->worktree_status;
>>  		break;
>> +	default:
>> +		die("BUG: unhandled change_type %d in wt_status_print_change_data",
>> +		    change_type);
>
> Micronit: s/unhandled/invalid/.

I actually think "unhandled" is more correct for this one; we may
add new change_type later in the caller, and we do not want to
forget to add a new case arm that handles the new value.