Hi Peff,
On Thu, 8 Sep 2016, Jeff King wrote:
> On Thu, Sep 08, 2016 at 09:31:11AM +0200, Johannes Schindelin wrote:
>
> > diff --git a/git-compat-util.h b/git-compat-util.h
> > index db89ba7..19128b3 100644
> > --- a/git-compat-util.h
> > +++ b/git-compat-util.h
> > @@ -965,6 +965,27 @@ void git_qsort(void *base, size_t nmemb, size_t size,
> > #define qsort git_qsort
> > #endif
> >
> > +static inline int regexec_buf(const regex_t *preg, const char *buf, size_t
> > size,
> > + size_t nmatch, regmatch_t pmatch[], int eflags)
> > +{
> > +#ifdef REG_STARTEND
> > + assert(nmatch > 0 && pmatch);
> > + pmatch[0].rm_so = 0;
> > + pmatch[0].rm_eo = size;
> > + return regexec(preg, buf, nmatch, pmatch, eflags | REG_STARTEND);
> > +#else
> > + char *buf2 = xmalloc(size + 1);
> > + int ret;
> > +
> > + memcpy(buf2, buf, size);
> > + buf2[size] = '\0';
>
> I mentioned elsewhere that I'd prefer we just push people into using
> compat/regex if they don't have REG_STARTEND. But if we _do_ keep this
> fallback, note that the above has a buffer overflow (think what happens
> when "size" is the maximum value for a size_t). You can avoid it by
> using xmallocz().
That buffer overflow does not exist: If size were the maximum value for
size_t, then buf->ptr would point at a buffer that occupies the entire
available memory, meaning that there is no space left for buf->ptr, let
alone for buf.
But I get your point. It is better to be consistent and use the same logic
for *all* allocations.
Ciao,
Dscho
