On Mon, Sep 26, 2016 at 10:10:46AM -0700, Junio C Hamano wrote:
> > struct disambiguate_state {
> > int len; /* length of prefix in hex chars */
> > - char hex_pfx[GIT_SHA1_HEXSZ];
> > + char hex_pfx[GIT_SHA1_HEXSZ + 1];
> > unsigned char bin_pfx[GIT_SHA1_RAWSZ];
> >
> > disambiguate_hint_fn fn;
> > @@ -291,7 +291,6 @@ static int init_object_disambiguation(const char *name, int len,
> > return -1;
> >
> > memset(ds, 0, sizeof(*ds));
> > - memset(ds->hex_pfx, 'x', GIT_SHA1_HEXSZ);
>
> As the whole thing is cleared here...
>
> >
> > for (i = 0; i < len ;i++) {
> > unsigned char c = name[i];
> > @@ -313,6 +312,7 @@ static int init_object_disambiguation(const char *name, int len,
> > }
> >
> > ds->len = len;
> > + ds->hex_pfx[len] = '\0';
>
> ... do we even need this one? It would not hurt, though.
Sharp eyes. I noticed that while writing it, but wondered if anybody
else would. :)
I left the second one in to make the intention more explicit, and so
readers did not have to worry that the NULs were overwritten in the
loop. I'd be OK with it either way, though.
-Peff