Re: [PATCH] Remove a dead assignment
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:19
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] Remove a dead assignment · Chris Wilson <hidden>
- 2016-06-15 · Re: [PATCH] Remove a dead assignment · Matthieu Moy <hidden>
- 2016-06-15 · Re: [PATCH] Remove a dead assignment · Michael Schubert <hidden>
- 2016-06-15 · [PATCH] Remove a dead assignment · Chris Wilson <hidden>
- 2016-06-15 · Re: [PATCH] Remove a dead assignment · Junio C Hamano <hidden>
Chris Wilson [off-list ref] 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.
quoted hunk
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); }