From 4db34cc134978012cf3f1b07de981a0418ed1523 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 Mar 2013 07:08:17 -0400 Subject: [PATCH 1/7] fast-import: use pointer-to-pointer to keep list tail This is shorter, idiomatic, and it means the compiler does not get confused about whether our "e" pointer is valid, letting us drop the "e = e" hack. Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- fast-import.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/fast-import.c b/fast-import.c index c2a814ec66..583a439dba 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2613,7 +2613,7 @@ static int parse_from(struct branch *b) static struct hash_list *parse_merge(unsigned int *count) { - struct hash_list *list = NULL, *n, *e = e; + struct hash_list *list = NULL, **tail = &list, *n; const char *from; struct branch *s; @@ -2641,11 +2641,9 @@ static struct hash_list *parse_merge(unsigned int *count) die("Invalid ref name or SHA1 expression: %s", from); n->next = NULL; - if (list) - e->next = n; - else - list = n; - e = n; + *tail = n; + tail = &n->next; + (*count)++; read_next_command(); } From cbfd5e1cbb539b82a34195efd0edde20d45a6439 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 Mar 2013 07:10:28 -0400 Subject: [PATCH 2/7] drop some obsolete "x = x" compiler warning hacks In cases where the setting and access of a variable are protected by the same conditional flag, older versions of gcc would generate a "might be used unitialized" warning. We silence the warning by initializing the variable to itself, a hack that gcc recognizes. Modern versions of gcc are smart enough to get this right, going back to at least version 4.3.5. gcc 4.1 does get it wrong in both cases, but is sufficiently old that we probably don't need to care about it anymore. Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 2 +- fast-import.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 00528ddc38..ad29000736 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -193,7 +193,7 @@ static int batch_one_object(const char *obj_name, int print_contents) unsigned char sha1[20]; enum object_type type = 0; unsigned long size; - void *contents = contents; + void *contents; if (!obj_name) return 1; diff --git a/fast-import.c b/fast-import.c index 583a439dba..e12a8b88ee 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2434,7 +2434,7 @@ static void note_change_n(struct branch *b, unsigned char *old_fanout) { const char *p = command_buf.buf + 2; static struct strbuf uq = STRBUF_INIT; - struct object_entry *oe = oe; + struct object_entry *oe; struct branch *s; unsigned char sha1[20], commit_sha1[20]; char path[60]; From c5d5c9a9a3e31b7749e1f7ddfc8825b935eda1eb Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 Mar 2013 07:13:33 -0400 Subject: [PATCH 3/7] transport: drop "int cmp = cmp" hack According to 47ec794, this initialization is meant to squelch an erroneous uninitialized variable warning from gcc 4.0.1. That version is quite old at this point, and gcc 4.1 and up handle it fine, with one exception. There seems to be a regression in gcc 4.6.3, which produces the warning; however, gcc versions 4.4.7 and 4.7.2 do not. Signed-off-by: Jeff King Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transport.c b/transport.c index 886ffd8b1e..87b8f145ac 100644 --- a/transport.c +++ b/transport.c @@ -106,7 +106,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list) return; for (;;) { - int cmp = cmp, len; + int cmp, len; if (!fgets(buffer, sizeof(buffer), f)) { fclose(f); From 25043d8aea7859497c12cb035e6688f76e32ac13 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 Mar 2013 11:45:00 -0400 Subject: [PATCH 4/7] run-command: always set failed_errno in start_command When we fail to fork, we set the failed_errno variable to the value of errno so it is not clobbered by later syscalls. However, we do so in a conditional, and it is hard to see later under what conditions the variable has a valid value. Instead of setting it only when fork fails, let's just always set it after forking. This is more obvious for human readers (as we are no longer setting it as a side effect of a strerror call), and it is more obvious to gcc, which no longer generates a spurious -Wuninitialized warning. It also happens to match what the WIN32 half of the #ifdef does. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- run-command.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/run-command.c b/run-command.c index 07e27ff4c8..765c2ce056 100644 --- a/run-command.c +++ b/run-command.c @@ -273,7 +273,7 @@ int start_command(struct child_process *cmd) { int need_in, need_out, need_err; int fdin[2], fdout[2], fderr[2]; - int failed_errno = failed_errno; + int failed_errno; char *str; /* @@ -341,6 +341,7 @@ fail_pipe: notify_pipe[0] = notify_pipe[1] = -1; cmd->pid = fork(); + failed_errno = errno; if (!cmd->pid) { /* * Redirect the channel to write syscall error messages to @@ -420,7 +421,7 @@ fail_pipe: } if (cmd->pid < 0) error("cannot fork() for %s: %s", cmd->argv[0], - strerror(failed_errno = errno)); + strerror(errno)); else if (cmd->clean_on_exit) mark_child_for_cleanup(cmd->pid); From 3aa99df802020f73981d1169c05375d5e2b49cc9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 Mar 2013 11:44:39 -0400 Subject: [PATCH 5/7] fast-import: clarify "inline" logic in file_change_m When we read a fast-import line like: M 100644 :1 foo.c we point the local object_entry variable "oe" to the object named by the mark ":1". When the input uses the "inline" construct, however, we do not have such an object_entry. The current code is careful not to access "oe" in the inline case, but we can make the assumption even more obvious (and catch violations of it) by setting oe to NULL and adding a comment. As a bonus, this also squelches an over-zealous gcc -Wuninitialized warning, which means we can drop the "oe = oe" initialization hack. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- fast-import.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fast-import.c b/fast-import.c index e12a8b88ee..a0c2c2ff14 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2265,7 +2265,7 @@ static void file_change_m(struct branch *b) const char *p = command_buf.buf + 2; static struct strbuf uq = STRBUF_INIT; const char *endp; - struct object_entry *oe = oe; + struct object_entry *oe; unsigned char sha1[20]; uint16_t mode, inline_data = 0; @@ -2292,6 +2292,7 @@ static void file_change_m(struct branch *b) hashcpy(sha1, oe->idx.sha1); } else if (!prefixcmp(p, "inline ")) { inline_data = 1; + oe = NULL; /* not used with inline_data, but makes gcc happy */ p += strlen("inline"); /* advance to space */ } else { if (get_sha1_hex(p, sha1)) From b8527d5fa61f4401c38e5992b154d7a323aacbf0 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 21 Mar 2013 07:05:28 -0400 Subject: [PATCH 6/7] wt-status: fix possible use of uninitialized variable In wt_status_print_change_data, we accept a change_type flag that is meant to be either WT_STATUS_UPDATED or WT_STATUS_CHANGED. We then switch() on this value to set the local variable "status" for each case, but do not provide a fallback "default" label to the switch statement. As a result, the compiler realizes that "status" might be unset, and complains with a warning. To silence this warning, we use the "int status = status" trick. This is correct with the current code, as all callers provide one of the two expected change_type flags. However, it's also a maintenance trap, as there is nothing to prevent future callers from passing another flag, nor to document this assumption. Instead of using the "x = x" hack, let's handle the default case in the switch() statement with a die("BUG"). That tells the compiler and any readers of the code exactly what the function's input assumptions are. We could also convert the flag to an enum, which would provide a compile-time check on the function input. However, since these flags are part of a larger enum, that would make the code unnecessarily complex (we would have to make a new enum with just the two flags, and then convert it to the old enum for passing to sub-functions). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- wt-status.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index ef405d03d9..7555817786 100644 --- a/wt-status.c +++ b/wt-status.c @@ -264,7 +264,7 @@ static void wt_status_print_change_data(struct wt_status *s, { struct wt_status_change_data *d = it->util; const char *c = color(change_type, s); - int status = status; + int status; char *one_name; char *two_name; const char *one, *two; @@ -292,6 +292,9 @@ static void wt_status_print_change_data(struct wt_status *s, } status = d->worktree_status; break; + default: + die("BUG: unhandled change_type %d in wt_status_print_change_data", + change_type); } one = quote_path(one_name, -1, &onebuf, s->prefix); From c9fc4415e2c7d7673cdad34d41114ede3435a395 Mon Sep 17 00:00:00 2001 From: Max Nanasy Date: Thu, 21 Mar 2013 12:53:38 -0700 Subject: [PATCH 7/7] diff.c: diff.renamelimit => diff.renameLimit in message In the warning message printed when rename or unmodified copy detection was skipped due to too many files, change "diff.renamelimit" to "diff.renameLimit", in order to make it consistent with git documentation, which consistently uses "diff.renameLimit". Signed-off-by: Max Nanasy Signed-off-by: Junio C Hamano --- diff.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff.c b/diff.c index 156fec4470..052974eb97 100644 --- a/diff.c +++ b/diff.c @@ -4662,7 +4662,7 @@ int diff_result_code(struct diff_options *opt, int status) { int result = 0; - diff_warn_rename_limit("diff.renamelimit", + diff_warn_rename_limit("diff.renameLimit", opt->needed_rename_limit, opt->degraded_cc_to_c); if (!DIFF_OPT_TST(opt, EXIT_WITH_STATUS) &&