Re: [PATCH] Remove a dead assignment

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> The if statement says "we might be passing NULL in fmt and in that case
> please fall back to user_format" to human readers, but the compiler is too
> stupid to infer such an intention, so you have to help it with your brain.
> I have to wonder if the strbuf_expand() should be passing fmt instead of
> user_format.  "git blame -L1082,+7 pretty.c" points at 5b16360 (pretty:
> Initialize notes if %N is used, 2010-04-13).
>
> The only callsite that is introduced by that patch passes NULL to fmt, so
> a better fix might be to do something like this instead.

If somebody cares about the reusability of the code for other callsites
added in the future, we could do this instead.

I think this is what Johannes wanted to do from the beginning, and is a
better fix than my previous one to remove the fmt parameter altogether.

-- >8 --
Subject: userformat_find_requirements(): find requirement for the correct format

This function was introduced in 5b16360 (pretty: Initialize notes if %N is
used, 2010-04-13) to check what kind of information the "log --format=..."
user format string wants. The function can be passed a NULL instead of a
format string to ask it to check user_format variable kept by an earlier
call to save_user_format().

But it unconditionally checked user_format and not the string it was
given.  The only caller introduced by the change passes NULL, which
kept the bug unnoticed, until a new GCC noticed that there is an
assignment to fmt that is never used.

Noticed-by: Chris Wilson's compiler
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---
 pretty.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/pretty.c b/pretty.c
index dff5c8d..52174fd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1084,7 +1084,7 @@ void userformat_find_requirements(const char *fmt, struct userformat_want *w)
 			return;
 		fmt = user_format;
 	}
-	strbuf_expand(&dummy, user_format, userformat_want_item, w);
+	strbuf_expand(&dummy, fmt, userformat_want_item, w);
 	strbuf_release(&dummy);
 }
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]