{"thread":{"id":"5416","subject":"[PATCH] Refactor sha1_pack_index_name and sha1_pack_name to use a common backend","startedAt":"2006-08-28T00:16:10Z","lastAt":"2006-08-28T13:34:44Z","messageCount":3,"participants":["Jonas Fonseca","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"26018","messageId":"20060828001610.GC20904@diku.dk","threadId":"5416","inReplyTo":null,"subject":"[PATCH] Refactor sha1_pack_index_name and sha1_pack_name to use a common backend","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2006-08-28T00:16:10Z","receivedAt":"2006-08-28T00:16:10Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Signed-off-by: Jonas Fonseca \n---\n\nTested with doing a local cg-clone, since there doesn't seem to be any\ntests of users, such as git-local-fetch, under t/.\n\nThis adds another pair of static buffers, if that's a problem and the\ncleanup is still wanted I can change it to use malloc.\n\n sha1_file.c | 64 +++++++++++++++++++++++------------------------------------\n 1 files changed, 25 insertions(+), 39 deletions(-)\n\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 01aa745..5a846f5 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -147,65 +147,51 @@ char *sha1_file_name(const unsigned char\n \treturn base;\n }\n \n-char *sha1_pack_name(const unsigned char *sha1)\n+static int fill_sha1_pack_name(const unsigned char *sha1, char base[], size_t baselen,\n+\t\t\t int dir_offset, const char *extension)\n {\n \tstatic const char hex[] = \"0123456789abcdef\";\n-\tstatic char *name, *base, *buf;\n-\tstatic const char *last_objdir;\n \tconst char *sha1_file_directory = get_object_directory();\n+\tchar *buf;\n \tint i;\n \n-\tif (!last_objdir || strcmp(last_objdir, sha1_file_directory)) {\n-\t\tint len = strlen(sha1_file_directory);\n-\t\tif (base)\n-\t\t\tfree(base);\n-\t\tbase = xmalloc(len + 60);\n-\t\tsprintf(base, \"%s/pack/pack-1234567890123456789012345678901234567890.pack\", sha1_file_directory);\n-\t\tname = base + len + 11;\n-\t\tif (last_objdir)\n-\t\t\tfree((char *) last_objdir);\n-\t\tlast_objdir = strdup(sha1_file_directory);\n+\tbase[dir_offset] = 0;\n+\tif (strcmp(base, sha1_file_directory)) {\n+\t\tdir_offset = strlen(sha1_file_directory);\n+\t\tif (snprintf(base, baselen,\n+\t\t\t \"%s/pack/pack-1234567890123456789012345678901234567890.%s\",\n+\t\t\t sha1_file_directory, extension) >= baselen)\n+\t\t\tdie(\"pack name too long\");\n \t}\n+\tbase[dir_offset] = '/';\n \n-\tbuf = name;\n+\tbuf = base + dir_offset + 11;\n \n \tfor (i = 0; i < 20; i++) {\n \t\tunsigned int val = *sha1++;\n+\n \t\t*buf++ = hex[val >> 4];\n \t\t*buf++ = hex[val & 0xf];\n \t}\n-\t\n-\treturn base;\n+\n+\treturn dir_offset;\n }\n \n char *sha1_pack_index_name(const unsigned char *sha1)\n {\n-\tstatic const char hex[] = \"0123456789abcdef\";\n-\tstatic char *name, *base, *buf;\n-\tstatic const char *last_objdir;\n-\tconst char *sha1_file_directory = get_object_directory();\n-\tint i;\n+\tstatic char base[PATH_MAX + 1];\n+\tstatic int offset;\n \n-\tif (!last_objdir || strcmp(last_objdir, sha1_file_directory)) {\n-\t\tint len = strlen(sha1_file_directory);\n-\t\tif (base)\n-\t\t\tfree(base);\n-\t\tbase = xmalloc(len + 60);\n-\t\tsprintf(base, \"%s/pack/pack-1234567890123456789012345678901234567890.idx\", sha1_file_directory);\n-\t\tname = base + len + 11;\n-\t\tif (last_objdir)\n-\t\t\tfree((char *) last_objdir);\n-\t\tlast_objdir = strdup(sha1_file_directory);\n-\t}\n+\toffset = fill_sha1_pack_name(sha1, base, sizeof(base), offset, \"idx\");\n+\treturn base;\n+}\n \n-\tbuf = name;\n+char *sha1_pack_name(const unsigned char *sha1)\n+{\n+\tstatic char base[PATH_MAX + 1];\n+\tstatic int offset;\n \n-\tfor (i = 0; i < 20; i++) {\n-\t\tunsigned int val = *sha1++;\n-\t\t*buf++ = hex[val >> 4];\n-\t\t*buf++ = hex[val & 0xf];\n-\t}\n-\t\n+\toffset = fill_sha1_pack_name(sha1, base, sizeof(base), offset, \"pack\");\n \treturn base;\n }\n \n-- \n1.4.2.g2f76-dirty\n\n-- \nJonas Fonseca\n"},{"id":"26028","messageId":"7vlkp9tquz.fsf@assigned-by-dhcp.cox.net","threadId":"5416","inReplyTo":"20060828001610.GC20904@diku.dk","subject":"Re: [PATCH] Refactor sha1_pack_index_name and sha1_pack_name to use a common backend","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-08-28T04:40:36Z","receivedAt":"2006-08-28T04:40:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonas Fonseca writes:\n\n> This adds another pair of static buffers, if that's a problem and the\n> cleanup is still wanted I can change it to use malloc.\n\nTo be perfectly honest, I would rather get rid of these two\nfunctions, and also sha1[] member from struct packed_git.\n\nThe real core git does not impose any limit to the name of the\npackfiles, other than that the name of the datafile ends with\n\".pack\" and that its corresponding pack index file has the same\nname ending with \".idx\". It is only the packfile support which\nwas later bolted on to commit walkers by commit bf592c5 that\nintroduced this silly expectation that they are always of form\n\"pack-[0-9a-f]{40}.pack\" and \"pack-[0-9a-f]{40}.idx\".\n\nFor example, fetch_pack() in local-fetch.c is given an object\nname and tries to find the packfile that contains it by looking\nat \".idx\" files downloaded from the other side. When it finds\none, it fabricates a name for the pack file by what is in\ntarget->sha1 (where target is of type \"struct packed_git\").\n\nHowever, struct packed_git already has perfectly good place that\nis meant to store the real name of the packfile, so this is\ntotally unnecessary.\n\nAs a clean-up, I'd rather see a patch that removes the need for\nthese two functions and one struct member, rather than keeping\nthese two misguided functions and consolidating their\nimplementations.\n"},{"id":"26043","messageId":"20060828133444.GA32266@diku.dk","threadId":"5416","inReplyTo":"7vlkp9tquz.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Refactor sha1_pack_index_name and sha1_pack_name to use a common backend","fromName":"Jonas Fonseca","fromEmail":"fonseca@diku.dk","sentAt":"2006-08-28T13:34:44Z","receivedAt":"2006-08-28T13:34:44Z","isPatch":true,"sender":{"key":"fonseca@diku.dk","avatar":"https://gravatar.com/avatar/f82f3ad698717c51873b020c750a92438c820a24056dc39fe4d07baa10a92264?d=mp&s=160"},"body":"Junio C Hamano wrote Sun, Aug 27, 2006:\n> Jonas Fonseca writes:\n> \n> > This adds another pair of static buffers, if that's a problem and the\n> > cleanup is still wanted I can change it to use malloc.\n> \n> As a clean-up, I'd rather see a patch that removes the need for\n> these two functions and one struct member, rather than keeping\n> these two misguided functions and consolidating their\n> implementations.\n\nThanks for the outline, I will look into removing this limitation.\n\n-- \nJonas Fonseca\n"}]}