Skip to content

Commit 8b0061b

Browse files
peffgitster
authored andcommitted
ref-filter: clarify lstrip/rstrip component counting
When a strip option to the %(refname) placeholder is asked to leave N path components, we first count up the path components to know how many to remove. That happens with a loop like this: /* Find total no of '/' separated path-components */ for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) ; which is a little hard to understand for two reasons. First, the dereference in "*p++" is seemingly useless, since nobody looks at the result. And static analyzers like Coverity will complain about that. But removing the "*" will cause gcc to complain with -Wint-conversion, since the two sides of the ternary do not match (one is a pointer and the other an int). Second, it is not clear what the meaning of "p" is at each iteration of the loop, as its position with respect to our walk over the string depends on how many slashes we've seen. The answer is that by itself, it doesn't really mean anything: "p + i" represents the current state of our walk, with "i" counting up slashes, and "p" by itself essentially meaningless. None of this behaves incorrectly, but ultimately the loop is just counting the slashes in the refname. We can do that much more simply with a for-loop iterating over the string and a separate slash counter. We can also drop the comment, which is somewhat misleading. We are counting slashes, not components (and a comment later in the function makes it clear that we must add one to compensate). In the new code it is obvious that we are counting slashes here. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent fe732a8 commit 8b0061b

1 file changed

Lines changed: 7 additions & 6 deletions

File tree

ref-filter.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2176,19 +2176,20 @@ static inline char *copy_advance(char *dst, const char *src)
21762176
static int normalize_component_count(const char *refname, int len)
21772177
{
21782178
if (len < 0) {
2179-
int i;
2180-
const char *p = refname;
2179+
int slashes = 0;
2180+
2181+
for (const char *p = refname; *p; p++) {
2182+
if (*p == '/')
2183+
slashes++;
2184+
}
21812185

2182-
/* Find total no of '/' separated path-components */
2183-
for (i = 0; p[i]; p[i] == '/' ? i++ : *p++)
2184-
;
21852186
/*
21862187
* The number of components we need to strip is now
21872188
* the total minus the components to be left (Plus one
21882189
* because we count the number of '/', but the number
21892190
* of components is one more than the no of '/').
21902191
*/
2191-
len = i + len + 1;
2192+
len = slashes + len + 1;
21922193
}
21932194
return len;
21942195
}

0 commit comments

Comments
 (0)