Skip to content

Commit 71571cd

Browse files
Martin Ågrengitster
authored andcommitted
sequencer: break out of loop explicitly
It came up in review [1, 2] that this non-idiomatic loop is a bit tricky. When we find a space, we set `len = i`, which gives us the answer we are looking for, but which also breaks out of the loop. It turns out that this loop can confuse compilers as well. My copy of gcc 7.3.0 realizes that we are essentially evaluating `(len + 1) < len` and warns that the behavior is undefined if `len` is `INT_MAX`. (Because the assignment `len = i` is guaranteed to decrease `len`, such undefined behavior is not actually possible.) Rewrite the loop to a more idiomatic variant which doesn't muck with `len` in the loop body. That should help compilers and human readers figure out what is going on here. But do note that we need to update `len` since it is not only used just after this loop (where we could have used `i` directly), but also later in this function. While at it, reduce the scope of `i`. [1] https://public-inbox.org/git/CAPig+cQbG2s-LrAo9+7C7=dXifbWFJ3SzuNa-QePHDk7egK=jg@mail.gmail.com/ [2] https://public-inbox.org/git/CAPig+cRjU6niXpT2FrDWZ0x1HmGf1ojVZj3uk2qXEGe-S7i_HQ@mail.gmail.com/ Helped-by: Eric Sunshine <[email protected]> Signed-off-by: Martin Ågren <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent cae598d commit 71571cd

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

sequencer.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2825,7 +2825,7 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)
28252825
struct tree_desc desc;
28262826
struct tree *tree;
28272827
struct unpack_trees_options unpack_tree_opts;
2828-
int ret = 0, i;
2828+
int ret = 0;
28292829

28302830
if (hold_locked_index(&lock, LOCK_REPORT_ON_ERROR) < 0)
28312831
return -1;
@@ -2845,10 +2845,13 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)
28452845
}
28462846
oidcpy(&oid, &opts->squash_onto);
28472847
} else {
2848+
int i;
2849+
28482850
/* Determine the length of the label */
28492851
for (i = 0; i < len; i++)
28502852
if (isspace(name[i]))
2851-
len = i;
2853+
break;
2854+
len = i;
28522855

28532856
strbuf_addf(&ref_name, "refs/rewritten/%.*s", len, name);
28542857
if (get_oid(ref_name.buf, &oid) &&

0 commit comments

Comments
 (0)