{"thread":{"id":"38567","subject":"[PATCH] hex.c: reduce memory footprint of sha1_to_hex static buffers","startedAt":"2015-02-13T21:18:49Z","lastAt":"2015-02-13T23:45:20Z","messageCount":5,"participants":["Stefan Beller","Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"256087","messageId":"1423862329-7899-1-git-send-email-sbeller@google.com","threadId":"38567","inReplyTo":null,"subject":"[PATCH] hex.c: reduce memory footprint of sha1_to_hex static buffers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-02-13T21:18:49Z","receivedAt":"2015-02-13T21:18:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"41 bytes is the exact number of bytes needed for having the returned\nhex string represented. 50 seems to be an arbitrary number, such\nthat there are no benefits from alignment to certain address boundaries.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n hex.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/hex.c b/hex.c\nindex 9ebc050..cfd9d72 100644\n--- a/hex.c\n+++ b/hex.c\n@@ -59,7 +59,7 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)\n char *sha1_to_hex(const unsigned char *sha1)\n {\n \tstatic int bufno;\n-\tstatic char hexbuffer[4][50];\n+\tstatic char hexbuffer[4][41];\n \tstatic const char hex[] = \"0123456789abcdef\";\n \tchar *buffer = hexbuffer[3 & ++bufno], *buf = buffer;\n \tint i;\n-- \n2.3.0.81.gc37f363\n"},{"id":"256089","messageId":"xmqq7fvlmqu8.fsf@gitster.dls.corp.google.com","threadId":"38567","inReplyTo":"1423862329-7899-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] hex.c: reduce memory footprint of sha1_to_hex static buffers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-13T21:41:51Z","receivedAt":"2015-02-13T21:41:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> 41 bytes is the exact number of bytes needed for having the returned\n> hex string represented. 50 seems to be an arbitrary number, such\n> that there are no benefits from alignment to certain address boundaries.\n\nYes, with s/seems to be/is/;\n\nThis comes from e83c5163 (Initial revision of \"git\", the information\nmanager from hell, 2005-04-07), and when dcb3450f (sha1_to_hex()\nusage cleanup, 2006-05-03) introduced the \"4 recycled buffers\" on\ntop, the underlying array was left at 50 bytes long.\n\nYou can now have \"I fixed Linus's bug\" badge ;-)\n\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  hex.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/hex.c b/hex.c\n> index 9ebc050..cfd9d72 100644\n> --- a/hex.c\n> +++ b/hex.c\n> @@ -59,7 +59,7 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)\n>  char *sha1_to_hex(const unsigned char *sha1)\n>  {\n>  \tstatic int bufno;\n> -\tstatic char hexbuffer[4][50];\n> +\tstatic char hexbuffer[4][41];\n>  \tstatic const char hex[] = \"0123456789abcdef\";\n>  \tchar *buffer = hexbuffer[3 & ++bufno], *buf = buffer;\n>  \tint i;\n"},{"id":"256091","messageId":"CAGZ79kaEun-mEQPg7cmPV7RJR8A+BxSsSpeAQgcsuJn8L4TGEg@mail.gmail.com","threadId":"38567","inReplyTo":"xmqq7fvlmqu8.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] hex.c: reduce memory footprint of sha1_to_hex static buffers","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-02-13T21:56:37Z","receivedAt":"2015-02-13T21:56:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Feb 13, 2015 at 1:41 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> 41 bytes is the exact number of bytes needed for having the returned\n>> hex string represented. 50 seems to be an arbitrary number, such\n>> that there are no benefits from alignment to certain address boundaries.\n>\n> Yes, with s/seems to be/is/;\n>\n> This comes from e83c5163 (Initial revision of \"git\", the information\n> manager from hell, 2005-04-07), and when dcb3450f (sha1_to_hex()\n> usage cleanup, 2006-05-03) introduced the \"4 recycled buffers\" on\n> top, the underlying array was left at 50 bytes long.\n>\n> You can now have \"I fixed Linus's bug\" badge ;-)\n\nI don't think it's a bug, it's just wasting memory?\n\nAs I could not find any documentation on the\nmagical 50 in the early days, I cc'd Linus\nin case there is something I did not think of yet.\n"},{"id":"256093","messageId":"CA+55aFyTcU0zjo9rTxetnRj72YS+Mo52sx85-Rk0woXEbQVfsw@mail.gmail.com","threadId":"38567","inReplyTo":"CAGZ79kaEun-mEQPg7cmPV7RJR8A+BxSsSpeAQgcsuJn8L4TGEg@mail.gmail.com","subject":"Re: [PATCH] hex.c: reduce memory footprint of sha1_to_hex static buffers","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2015-02-13T21:58:42Z","receivedAt":"2015-02-13T21:58:42Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"On Fri, Feb 13, 2015 at 1:56 PM, Stefan Beller <sbeller@google.com> wrote:\n>\n> As I could not find any documentation on the\n> magical 50 in the early days, I cc'd Linus\n> in case there is something I did not think of yet.\n\nNothing magical, it's just \"rounded up from 40 + NUL character\".\n\n                        Linus\n"},{"id":"256097","messageId":"xmqqtwypl6jz.fsf@gitster.dls.corp.google.com","threadId":"38567","inReplyTo":"CAGZ79kaEun-mEQPg7cmPV7RJR8A+BxSsSpeAQgcsuJn8L4TGEg@mail.gmail.com","subject":"Re: [PATCH] hex.c: reduce memory footprint of sha1_to_hex static buffers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-13T23:45:20Z","receivedAt":"2015-02-13T23:45:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> On Fri, Feb 13, 2015 at 1:41 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Stefan Beller <sbeller@google.com> writes:\n>>\n>>> 41 bytes is the exact number of bytes needed for having the returned\n>>> hex string represented. 50 seems to be an arbitrary number, such\n>>> that there are no benefits from alignment to certain address boundaries.\n>>\n>> Yes, with s/seems to be/is/;\n>>\n>> This comes from e83c5163 (Initial revision of \"git\", the information\n>> manager from hell, 2005-04-07), and when dcb3450f (sha1_to_hex()\n>> usage cleanup, 2006-05-03) introduced the \"4 recycled buffers\" on\n>> top, the underlying array was left at 50 bytes long.\n>>\n>> You can now have \"I fixed Linus's bug\" badge ;-)\n>\n> I don't think it's a bug, it's just wasting memory?\n\nYes and no ;-)  As I already said above, 50 \"is\" just an arbitrary\nnumber that is round and enough to hold 40 bytes with trailing NUL,\nand the waste does not lead to behaviour that is different from what\nwas intended, of course, so it would not crash.\n\nHowever, the wastage was bothersome enough to make you send a patch,\nso you can call it a \"bug\".  It was wasting readers time ;-)\n"}]}