Re: [PATCH] diff: remove ternary operator evaluating always to true

From: Stefan Beller <stefanbeller@googlemail.com>
Date: Aug 12, 2013, 08:38 UTC
Message-ID: <52089F22.6010106@googlemail.com>
In-Reply-To: <52089D81.5010506@googlemail.com>

On 08/12/2013 10:32 AM, Stefan Beller wrote:

Show 19 quoted lines

> 
> diff --git a/diff.c b/diff.c
> index e53ddad..de21971 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2254,8 +2254,11 @@ static void builtin_diff(const char *name_a,
>  			(!two->mode || S_ISGITLINK(two->mode))) {
>  		const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
>  		const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
> -		show_submodule_summary(o->file, one ? one->path : two->path,
> -				line_prefix,
> +		struct diff_filespec *spec = one && DIFF_FILE_VALID(one) ? one : two;
> +		if (!spec && !DIFF_FILE_VALID(spec))
> +			die("BUG: two invalid diff_filespec structs in diff");
> +
> +		show_submodule_summary(o->file, spec->path, line_prefix,
>  				one->sha1, two->sha1, two->dirty_submodule,
>  				meta, del, add, reset);
>  		return;

This doesn't make sense, as we're definitely dereferencing both one and two with ->sha1, so we do not need the "one &&" and the "!spec &&". I think the originally sent patches are correct, but the commit message may need rewriting to comply with Jeffs nitpicking.