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

[Stefan Beller <stefanbeller@googlemail.com>]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

On 08/12/2013 10:32 AM, Stefan Beller wrote:
> 
> 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.




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]