On Sun, Oct 23, 2016 at 11:00:48AM +0200, René Scharfe wrote: > Overflow is defined for unsigned integers, but not for signed ones. > Make the ring buffer index in sha1_to_hex() unsigned to be on the > safe side. > > Signed-off-by: Rene Scharfe <l....@web.de> > --- > Hard to trigger, but probably even harder to diagnose once someone > somehow manages to do it on some uncommon architecture.
Indeed. If we are worried about overflow, we would also want to assume that it wraps at a multiple of 4, but that is probably a sane assumption. > diff --git a/hex.c b/hex.c > index ab2610e..8c6c189 100644 > --- a/hex.c > +++ b/hex.c > @@ -76,7 +76,7 @@ char *oid_to_hex_r(char *buffer, const struct object_id > *oid) > > char *sha1_to_hex(const unsigned char *sha1) > { > - static int bufno; > + static unsigned int bufno; > static char hexbuffer[4][GIT_SHA1_HEXSZ + 1]; > return sha1_to_hex_r(hexbuffer[3 & ++bufno], sha1); > } I wonder if just truncating bufno would be conceptually simpler (albeit longer): bufno++; bufno &= 3; return sha1_to_hex_r(hexbuffer[bufno], sha1); You could also write the second line like: bufno %= ARRAY_SIZE(hexbuffer); which is less magical (right now the set of buffers must be a power of 2). I expect the compiler could turn that into a bitmask itself. I'm fine with any of the options. I guess you'd want a similar patch for find_unique_abbrev on top of jk/no-looking-at-dotgit-outside-repo. -Peff