sha1_to_hex() usage cleanup

sha1_to_hex() usage cleanup

[Linus Torvalds]

Somebody on the #git channel complained that the sha1_to_hex() thing uses 
a static buffer which caused an error message to show the same hex output 
twice instead of showing two different ones.

That's pretty easily rectified by making it uses a simple LRU of a few 
buffers, which also allows some other users (that were aware of the buffer 
re-use) to be written in a more straightforward manner.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
---

This is another throw-away patch of mine. Not a big deal, but since I 
tried it, I might as well try to submit it and see if Junio agrees..

diff --git a/diff.c b/diff.c
index 6762fce..c845c87 100644
--- a/diff.c
+++ b/diff.c
@@ -1018,14 +1018,12 @@ static void run_diff(struct diff_filepai
 	}
 
 	if (memcmp(one->sha1, two->sha1, 20)) {
-		char one_sha1[41];
 		int abbrev = o->full_index ? 40 : DEFAULT_ABBREV;
-		memcpy(one_sha1, sha1_to_hex(one->sha1), 41);
 
 		len += snprintf(msg + len, sizeof(msg) - len,
 				"index %.*s..%.*s",
-				abbrev, one_sha1, abbrev,
-				sha1_to_hex(two->sha1));
+				abbrev, sha1_to_hex(one->sha1),
+				abbrev, sha1_to_hex(two->sha1));
 		if (one->mode == two->mode)
 			len += snprintf(msg + len, sizeof(msg) - len,
 					" %06o", one->mode);
diff --git a/merge-tree.c b/merge-tree.c
index 50528d5..cc7b5bd 100644
--- a/merge-tree.c
+++ b/merge-tree.c
@@ -24,16 +24,14 @@ static const char *sha1_to_hex_zero(cons
 
 static void resolve(const char *base, struct name_entry *branch1, struct name_entry *result)
 {
-	char branch1_sha1[50];
-
 	/* If it's already branch1, don't bother showing it */
 	if (!branch1)
 		return;
-	memcpy(branch1_sha1, sha1_to_hex_zero(branch1->sha1), 41);
 
 	printf("0 %06o->%06o %s->%s %s%s\n",
 		branch1->mode, result->mode,
-		branch1_sha1, sha1_to_hex_zero(result->sha1),
+		sha1_to_hex_zero(branch1->sha1),
+		sha1_to_hex_zero(result->sha1),
 		base, result->path);
 }
 
diff --git a/sha1_file.c b/sha1_file.c
index f2d33af..5464828 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -108,9 +108,10 @@ int safe_create_leading_directories(char
 
 char * sha1_to_hex(const unsigned char *sha1)
 {
-	static char buffer[50];
+	static int bufno;
+	static char hexbuffer[4][50];
 	static const char hex[] = "0123456789abcdef";
-	char *buf = buffer;
+	char *buffer = hexbuffer[3 & ++bufno], *buf = buffer;
 	int i;
 
 	for (i = 0; i < 20; i++) {

Re: sha1_to_hex() usage cleanup

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Somebody on the #git channel complained that the sha1_to_hex() thing uses 
> a static buffer which caused an error message to show the same hex output 
> twice instead of showing two different ones.
>
> That's pretty easily rectified by making it uses a simple LRU of a few 
> buffers, which also allows some other users (that were aware of the buffer 
> re-use) to be written in a more straightforward manner.
>
> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
> ---
>
> This is another throw-away patch of mine. Not a big deal, but since I 
> tried it, I might as well try to submit it and see if Junio agrees..

Makes sort of sense in that the callers still need to be aware
of the magic 4 limit but as long as they are they do not have to
worry about allocation/deallocation/copying.  But is that kind
of cheat maintainable?  I dunno.

Re: sha1_to_hex() usage cleanup

[Linus Torvalds]

On Wed, 3 May 2006, Junio C Hamano wrote:
> 
> Makes sort of sense in that the callers still need to be aware
> of the magic 4 limit but as long as they are they do not have to
> worry about allocation/deallocation/copying.  But is that kind
> of cheat maintainable?  I dunno.

If we just tell people that the rule is that it's still a statically 
allocated buffer, but that you can magically use two "sha1_to_hex()" calls 
in the same printf(), we'll probably get a good mix of "usable and safe".

		Linus