Re: [PATCH 1/4] wt-status: fix possible use of uninitialized variable
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 21, 2013, 19:55 UTC
- Message-ID
- <7vip4ka5pb.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20130321194949.GG29311@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 35 quoted lines
> 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.