{"thread":{"id":"4046","subject":"sha1_to_hex() usage cleanup","startedAt":"2006-05-04T00:21:08Z","lastAt":"2006-05-04T01:27:09Z","messageCount":3,"participants":["Linus Torvalds","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"19482","messageId":"Pine.LNX.4.64.0605031717190.4086@g5.osdl.org","threadId":"4046","inReplyTo":null,"subject":"sha1_to_hex() usage cleanup","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-04T00:21:08Z","receivedAt":"2006-05-04T00:21:08Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nSomebody on the #git channel complained that the sha1_to_hex() thing uses \na static buffer which caused an error message to show the same hex output \ntwice instead of showing two different ones.\n\nThat's pretty easily rectified by making it uses a simple LRU of a few \nbuffers, which also allows some other users (that were aware of the buffer \nre-use) to be written in a more straightforward manner.\n\nSigned-off-by: Linus Torvalds \n---\n\nThis is another throw-away patch of mine. Not a big deal, but since I \ntried it, I might as well try to submit it and see if Junio agrees..\n\ndiff --git a/diff.c b/diff.c\nindex 6762fce..c845c87 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1018,14 +1018,12 @@ static void run_diff(struct diff_filepai\n \t}\n \n \tif (memcmp(one->sha1, two->sha1, 20)) {\n-\t\tchar one_sha1[41];\n \t\tint abbrev = o->full_index ? 40 : DEFAULT_ABBREV;\n-\t\tmemcpy(one_sha1, sha1_to_hex(one->sha1), 41);\n \n \t\tlen += snprintf(msg + len, sizeof(msg) - len,\n \t\t\t\t\"index %.*s..%.*s\",\n-\t\t\t\tabbrev, one_sha1, abbrev,\n-\t\t\t\tsha1_to_hex(two->sha1));\n+\t\t\t\tabbrev, sha1_to_hex(one->sha1),\n+\t\t\t\tabbrev, sha1_to_hex(two->sha1));\n \t\tif (one->mode == two->mode)\n \t\t\tlen += snprintf(msg + len, sizeof(msg) - len,\n \t\t\t\t\t\" %06o\", one->mode);\ndiff --git a/merge-tree.c b/merge-tree.c\nindex 50528d5..cc7b5bd 100644\n--- a/merge-tree.c\n+++ b/merge-tree.c\n@@ -24,16 +24,14 @@ static const char *sha1_to_hex_zero(cons\n \n static void resolve(const char *base, struct name_entry *branch1, struct name_entry *result)\n {\n-\tchar branch1_sha1[50];\n-\n \t/* If it's already branch1, don't bother showing it */\n \tif (!branch1)\n \t\treturn;\n-\tmemcpy(branch1_sha1, sha1_to_hex_zero(branch1->sha1), 41);\n \n \tprintf(\"0 %06o->%06o %s->%s %s%s\\n\",\n \t\tbranch1->mode, result->mode,\n-\t\tbranch1_sha1, sha1_to_hex_zero(result->sha1),\n+\t\tsha1_to_hex_zero(branch1->sha1),\n+\t\tsha1_to_hex_zero(result->sha1),\n \t\tbase, result->path);\n }\n \ndiff --git a/sha1_file.c b/sha1_file.c\nindex f2d33af..5464828 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -108,9 +108,10 @@ int safe_create_leading_directories(char\n \n char * sha1_to_hex(const unsigned char *sha1)\n {\n-\tstatic char buffer[50];\n+\tstatic int bufno;\n+\tstatic char hexbuffer[4][50];\n \tstatic const char hex[] = \"0123456789abcdef\";\n-\tchar *buf = buffer;\n+\tchar *buffer = hexbuffer[3 & ++bufno], *buf = buffer;\n \tint i;\n \n \tfor (i = 0; i < 20; i++) {\n"},{"id":"19485","messageId":"7v1wvabp7a.fsf@assigned-by-dhcp.cox.net","threadId":"4046","inReplyTo":"Pine.LNX.4.64.0605031717190.4086@g5.osdl.org","subject":"Re: sha1_to_hex() usage cleanup","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-04T00:36:57Z","receivedAt":"2006-05-04T00:36:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Somebody on the #git channel complained that the sha1_to_hex() thing uses \n> a static buffer which caused an error message to show the same hex output \n> twice instead of showing two different ones.\n>\n> That's pretty easily rectified by making it uses a simple LRU of a few \n> buffers, which also allows some other users (that were aware of the buffer \n> re-use) to be written in a more straightforward manner.\n>\n> Signed-off-by: Linus Torvalds \n> ---\n>\n> This is another throw-away patch of mine. Not a big deal, but since I \n> tried it, I might as well try to submit it and see if Junio agrees..\n\nMakes sort of sense in that the callers still need to be aware\nof the magic 4 limit but as long as they are they do not have to\nworry about allocation/deallocation/copying. But is that kind\nof cheat maintainable? I dunno.\n"},{"id":"19493","messageId":"Pine.LNX.4.64.0605031826070.4086@g5.osdl.org","threadId":"4046","inReplyTo":"7v1wvabp7a.fsf@assigned-by-dhcp.cox.net","subject":"Re: sha1_to_hex() usage cleanup","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-04T01:27:09Z","receivedAt":"2006-05-04T01:27:09Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Wed, 3 May 2006, Junio C Hamano wrote:\n> \n> Makes sort of sense in that the callers still need to be aware\n> of the magic 4 limit but as long as they are they do not have to\n> worry about allocation/deallocation/copying. But is that kind\n> of cheat maintainable? I dunno.\n\nIf we just tell people that the rule is that it's still a statically \nallocated buffer, but that you can magically use two \"sha1_to_hex()\" calls \nin the same printf(), we'll probably get a good mix of \"usable and safe\".\n\n\t\tLinus\n"}]}