From: Jonathan Nieder <jrnieder@gmail.com>
Date: Thu, 21 Mar 2013 19:49:50 GMT
Subject: Re: [PATCH 1/4] wt-status: fix possible use of uninitialized variable
Message-ID: <20130321194949.GG29311@google.com>
In-Reply-To: <20130321110527.GA18819@sigill.intra.peff.net>

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/.

Thanks,
Jonathan