On Sat, Jan 4, 2025 at 9:26 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
> > for (cp = name, bracket_depth = 0; *cp; cp++) {
> > - if (*cp == '{')
> > + if (*(cp+1) == '{' && (*cp == '@' || *cp == '^')) {
> > + cp++;
> > bracket_depth++;
>
> Checking cp[1] before even knowing if cp[0] is the end of the string
> (hence cp[1] is an out of bounds access) smells fishy.
We checked *cp in the loop already, so we know cp[0] != '\0'.
Combined with the fact that this is a NUL-terminated string, we
therefore also know that cp[1] is not an out-of-bounds access.
> If it were
> something like ...
>
> if (cp[0] && strchr("@^", cp[0]) && cp[1] == '{')
Since we know cp[0] != '\0' already, couldn't this be simplified to
if (strchr("@^", *cp) && cp[1] == '{')
?
I do like this form better though, yes.
> ... it may be a bit more palatable, perhaps? At least writing it
> this way we can easily scale when we find the third character we
> need to special case, hopefully, but again, I do prefer if we can
> find a solution that does not have such an intimate knowledge about
> "@^", which I just failed to do here X-<.
Yeah, I have failed to come up with an alternative as well. If I and
others can't come up with something better in a few days, I'll
resubmit with the above change and a comment in the commit message
that we'd prefer something better but were unable to come up with
anything.
