Re: [PATCH 1/4] ref-filter: factor out refname component counting
From: Karthik Nayak <hidden>
Date: 2026-02-22 17:04:50
Jeff King [off-list ref] writes:
On Tue, Feb 17, 2026 at 10:07:29AM -0800, Junio C Hamano wrote:quoted
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.
I'm embarrassed and frankly don't remember this code :) Your new patch looks sensible to me.
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
Attachments
- signature.asc [application/pgp-signature] 690 bytes