Re: [PATCH 1/4] ref-filter: factor out refname component counting
From: Jeff King <hidden>
Date: 2026-02-19 11:21:59
On Tue, Feb 17, 2026 at 10:07:29AM -0800, Junio C Hamano wrote:
Jeff King [off-list ref] writes:quoted
+ if (len < 0) { + int i; + const char *p = refname; + + /* Find total no of '/' separated path-components */ + for (i = 0; p[i]; p[i] == '/' ? i++ : *p++) + ;Sorry, but I have no idea what this loop (copied verbatim from the original) is trying to do. We start at the beginning of the refname string, and while we are in the leading run of '/' we increment i to find the end of that run. E.g., we start with refname="///foo", p points at the leftmost '/', i runs from 0 to 3 at which point p[i] points at the first non-'/' character, at which point we do *p++, to make p point at the second slash? Is the dereferencing of the pointer in *p++ a no-op that is there only to confuse readers? And then p moves to the right until p[i] points at the end of the string. It does count the number of slashes in 'i', but there is no satisfying simple answer to this question: "what does p mean while this loop runs?". Anyway, the conversion looks very faithful to the original.
Heh, I missed your message initially but was independently staring at
this because Coverity complained that the dereference in "*p++" is
useless. Which is...kind of right. It is a void context, so the
dereferenced char goes nowhere and it is a noop. But if you don't do it,
then gcc complains that the two sides of the ternary have mis-matched
types (an int and a pointer). Which is true, but since nobody looks at
the result, it does not matter.
Writing it like:
int i = 0;
while (p[i]) {
if (p[i] == '/')
i++;
else
p++;
}
perhaps resolves the syntactic confusion. Leaving only the semantic
confusion. ;)
I guess the thinking was that "p+i" represents the traversal, with "i"
encoding the counted slashes (so we must increment _one_ of them each
time). But I cannot fathom how that is easier than counting the slashes
like:
int slashes = 0;
for (p = refname; *p; p++) {
if (*p == '/')
slashes++;
}
Which made me wonder if I am missing some corner case, and it is not
just counting slashes. But it must be, because "i" is never incremented
except when we see a slash.
+cc Karthik, the original author, for any wisdom, but the commit is now
almost 10 years old.
Is it worth rewriting to the "slashes" form above for clarity? I was
afraid to touch it just to shut up Coverity, but now we have two
confused people.
-Peff