Re: [PATCH] Remove a dead assignment
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 25, 2011, 19:11 UTC
- Message-ID
- <7v4o4i4mte.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20110525184514.GA20005@localhost>
Chris Wilson <cwilson@vigilantsw.com> writes:
> Thanks! Well, I wasn't going to report this dead assignment since > it wasn't done recently, but now I want to figure out how to properly > submit a patch. :) Am I there yet? and thanks for the help.
The compiler does not understand the meaning of the code, so after seeing such a "set but unused" statement, you should wonder why such a seemingly useless statement is there, before sending a mechanical patch to remove it without thinking things through.
Show 15 quoted lines
> pretty.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/pretty.c b/pretty.c
> index dff5c8d..5667c7f 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1082,7 +1082,6 @@ void userformat_find_requirements(const char *fmt, struct userformat_
> if (!fmt) {
> if (!user_format)
> return;
> - fmt = user_format;
> }
> strbuf_expand(&dummy, user_format, userformat_want_item, w);
> strbuf_release(&dummy);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.
builtin/log.c | 3 +-- commit.h | 2 +- pretty.c | 10 ++++------ 3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c index f621990..9e05d46 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -110,8 +110,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix, if (argc > 1) die("unrecognized argument: %s", argv[1]); - memset(&w, 0, sizeof(w)); - userformat_find_requirements(NULL, &w); + userformat_find_requirements(&w); if (!rev->show_notes_given && (!rev->pretty_given || w.notes)) rev->show_notes = 1; diff --git a/commit.h b/commit.h index 3114bd1..b652c22 100644 --- a/commit.h +++ b/commit.h @@ -92,7 +92,7 @@ extern char *reencode_commit_message(const struct commit *commit, extern void get_commit_format(const char *arg, struct rev_info *); extern const char *format_subject(struct strbuf *sb, const char *msg, const char *line_separator); -extern void userformat_find_requirements(const char *fmt, struct userformat_want *w); +extern void userformat_find_requirements(struct userformat_want *w); extern void format_commit_message(const struct commit *commit, const char *format, struct strbuf *sb, const struct pretty_print_context *context); diff --git a/pretty.c b/pretty.c index dff5c8d..ca24925 100644 --- a/pretty.c +++ b/pretty.c @@ -1075,15 +1075,13 @@ static size_t userformat_want_item(struct strbuf *sb, const char *placeholder, return 0; } -void userformat_find_requirements(const char *fmt, struct userformat_want *w) +void userformat_find_requirements(struct userformat_want *w) { struct strbuf dummy = STRBUF_INIT; - if (!fmt) { - if (!user_format) - return; - fmt = user_format; - } + memset(w, 0, sizeof(*w)); + if (!user_format) + return; strbuf_expand(&dummy, user_format, userformat_want_item, w); strbuf_release(&dummy); }