Re: [PATCH] sha1_name: make wraparound of the index into ring-buffer explicit

2016-11-01 Thread Jeff King
On Tue, Nov 01, 2016 at 09:49:07AM +0100, René Scharfe wrote:

> Overflow is defined for unsigned integers, but not for signed ones.
> Wrap around explicitly for the new ring-buffer in find_unique_abbrev()
> as we did in bb84735c for the ones in sha1_to_hex() and get_pathname(),
> thus avoiding signed overflows and getting rid of the magic number 3.

Thanks. I didn't mean to make you do this, but my procrastination won
again.

> diff --git a/sha1_name.c b/sha1_name.c
> index 06409a3..73a915f 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -510,7 +510,8 @@ const char *find_unique_abbrev(const unsigned char *sha1, 
> int len)
>  {
>   static int bufno;
>   static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
> - char *hex = hexbuffer[3 & ++bufno];
> + char *hex = hexbuffer[bufno];
> + bufno = (bufno + 1) % ARRAY_SIZE(hexbuffer);

Code looks obviously correct (and it even does the "start at 0" thing).

-Peff


[PATCH] sha1_name: make wraparound of the index into ring-buffer explicit

2016-11-01 Thread René Scharfe
Overflow is defined for unsigned integers, but not for signed ones.
Wrap around explicitly for the new ring-buffer in find_unique_abbrev()
as we did in bb84735c for the ones in sha1_to_hex() and get_pathname(),
thus avoiding signed overflows and getting rid of the magic number 3.

Signed-off-by: Rene Scharfe 
---
 sha1_name.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sha1_name.c b/sha1_name.c
index 06409a3..73a915f 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -510,7 +510,8 @@ const char *find_unique_abbrev(const unsigned char *sha1, 
int len)
 {
static int bufno;
static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
-   char *hex = hexbuffer[3 & ++bufno];
+   char *hex = hexbuffer[bufno];
+   bufno = (bufno + 1) % ARRAY_SIZE(hexbuffer);
find_unique_abbrev_r(hex, sha1, len);
return hex;
 }
-- 
2.10.2