Re: [PATCH 07/18] convert trivial cases to FLEX_ARRAY macros

From: Jeff King <peff@peff.net>
Date: Feb 16, 2016, 04:22 UTC
Message-ID: <20160216042222.GA27060@sigill.intra.peff.net>
In-Reply-To: <CAPig+cQOF7CsJNoiu8FAMk+csrOWG2dbEyxqfpWNwvGdUsjxcw@mail.gmail.com>

On Mon, Feb 15, 2016 at 11:18:56PM -0500, Eric Sunshine wrote:

Show 14 quoted lines

> > diff --git a/builtin/reflog.c b/builtin/reflog.c
> > @@ -408,13 +407,12 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
> >                 reflog_expire_cfg_tail = &reflog_expire_cfg;
> >
> >         for (ent = reflog_expire_cfg; ent; ent = ent->next)
> > -               if (ent->len == len &&
> > -                   !memcmp(ent->pattern, pattern, len))
> > +               if (!strncmp(ent->pattern, pattern, len) &&
> > +                   ent->pattern[len] == '\0')
> 
> If 'ent->pattern' is shorter than 'pattern' then the strncmp() will
> fail, thus it will short-circuit before ent->pattern[len] has a chance
> to access beyond the end of memory allocated for 'ent->pattern'. Okay,
> makes sense.

Yeah. It took me a minute to convince myself that this was correct. If you have a shorter or more clear way of writing it, I'm open to it. The best I could come up with is running an extra "strlen" and otherwise keeping the loop as it is; the performance on that is not as good, but if performance is a concern, maybe something besides a linear search would be in order. :)

-Peff