{"thread":{"id":"5198","subject":"[PATCH 7/9] git-verify-pack: buffer overrun paranoia","startedAt":"2006-08-10T15:02:29Z","lastAt":"2006-08-10T21:47:18Z","messageCount":18,"participants":["Rene Scharfe","Johannes Schindelin","Fredrik Kuivinen"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"25096","messageId":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":null,"subject":"[PATCH 0/9] git-verify-pack: clean up and make builtin","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:29Z","receivedAt":"2006-08-10T15:02:29Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This patch series makes git-verify-pack handle user supplied parameters\nmuch more carefully and converts it to a builtin command.  As a bonus it\nplugs a memory leak and makes some comments clearer.  It applies to the\ncurrent next branch (a934882).\n"},{"id":"25094","messageId":"11552221581171-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 1/9] Add has_extension()","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:30Z","receivedAt":"2006-08-10T15:02:30Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The little helper has_extension() documents through its name what we are\ntrying to do and makes sure we don't forget the underrun check.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n git-compat-util.h |    6 ++++++\n help.c            |    2 +-\n http-fetch.c      |    2 +-\n index-pack.c      |    2 +-\n local-fetch.c     |    2 +-\n refs.c            |    2 +-\n sha1_file.c       |    2 +-\n verify-pack.c     |    2 +-\n 8 files changed, 13 insertions(+), 7 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 3bcf5b1..dd92093 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -139,6 +139,12 @@ static inline ssize_t xwrite(int fd, con\n \t}\n }\n \n+static inline int has_extension(const char *filename, int len, const char *ext)\n+{\n+\tint extlen = strlen(ext);\n+\treturn len > extlen && !memcmp(filename + len - extlen, ext, extlen);\n+}\n+\n /* Sane ctype - no locale, and works with signed chars */\n #undef isspace\n #undef isdigit\ndiff --git a/help.c b/help.c\nindex fb731cc..7a7f775 100644\n--- a/help.c\n+++ b/help.c\n@@ -140,7 +140,7 @@ static void list_commands(const char *ex\n \t\t\tcontinue;\n \n \t\tentlen = strlen(de->d_name);\n-\t\tif (4 < entlen && !strcmp(de->d_name + entlen - 4, \".exe\"))\n+\t\tif (has_extension(de->d_name, entlen, \".exe\"))\n \t\t\tentlen -= 4;\n \n \t\tif (longest < entlen)\ndiff --git a/http-fetch.c b/http-fetch.c\nindex 36af3e5..6ea39f0 100644\n--- a/http-fetch.c\n+++ b/http-fetch.c\n@@ -870,7 +870,7 @@ static void process_ls_pack(struct remot\n \n \tif (strlen(ls->dentry_name) == 63 &&\n \t    !strncmp(ls->dentry_name, \"objects/pack/pack-\", 18) &&\n-\t    !strncmp(ls->dentry_name+58, \".pack\", 5)) {\n+\t    has_extension(ls->dentry_name, 63, \".pack\")) {\n \t\tget_sha1_hex(ls->dentry_name + 18, sha1);\n \t\tsetup_index(ls->repo, sha1);\n \t}\ndiff --git a/index-pack.c b/index-pack.c\nindex b39953d..a91e39e 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -447,7 +447,7 @@ int main(int argc, char **argv)\n \t\tusage(index_pack_usage);\n \tif (!index_name) {\n \t\tint len = strlen(pack_name);\n-\t\tif (len < 5 || strcmp(pack_name + len - 5, \".pack\"))\n+\t\tif (!has_extension(pack_name, len, \".pack\"))\n \t\t\tdie(\"packfile name '%s' does not end with '.pack'\",\n \t\t\t    pack_name);\n \t\tindex_name_buf = xmalloc(len);\ndiff --git a/local-fetch.c b/local-fetch.c\nindex 4bf86fb..59af57c 100644\n--- a/local-fetch.c\n+++ b/local-fetch.c\n@@ -44,7 +44,7 @@ static int setup_indices(void)\n \twhile ((de = readdir(dir)) != NULL) {\n \t\tint namelen = strlen(de->d_name);\n \t\tif (namelen != 50 || \n-\t\t    strcmp(de->d_name + namelen - 5, \".pack\"))\n+\t\t    !has_extension(de->d_name, namelen, \".pack\"))\n \t\t\tcontinue;\n \t\tget_sha1_hex(de->d_name + 5, sha1);\n \t\tsetup_index(sha1);\ndiff --git a/refs.c b/refs.c\nindex 02850b6..b01835f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -147,7 +147,7 @@ static int do_for_each_ref(const char *b\n \t\t\tnamelen = strlen(de->d_name);\n \t\t\tif (namelen > 255)\n \t\t\t\tcontinue;\n-\t\t\tif (namelen>5 && !strcmp(de->d_name+namelen-5,\".lock\"))\n+\t\t\tif (has_extension(de->d_name, namelen, \".lock\"))\n \t\t\t\tcontinue;\n \t\t\tmemcpy(path + baselen, de->d_name, namelen+1);\n \t\t\tif (stat(git_path(\"%s\", path), &st) < 0)\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 3d7a7d4..f0a4a7e 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -608,7 +608,7 @@ static void prepare_packed_git_one(char \n \t\tint namelen = strlen(de->d_name);\n \t\tstruct packed_git *p;\n \n-\t\tif (strcmp(de->d_name + namelen - 4, \".idx\"))\n+\t\tif (!has_extension(de->d_name, namelen, \".idx\"))\n \t\t\tcontinue;\n \n \t\t/* we have .idx.  Is it a file we can map? */\ndiff --git a/verify-pack.c b/verify-pack.c\nindex c99db9d..ef00204 100644\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -10,7 +10,7 @@ static int verify_one_pack(char *arg, in\n \t\t/* Should name foo.idx, but foo.pack may be named;\n \t\t * convert it to foo.idx\n \t\t */\n-\t\tif (!strcmp(arg + len - 5, \".pack\")) {\n+\t\tif (has_extension(arg, len, \".pack\")) {\n \t\t\tstrcpy(arg + len - 5, \".idx\");\n \t\t\tlen--;\n \t\t}\n-- \n1.4.2.rc2.g822a\n"},{"id":"25095","messageId":"11552221581434-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 2/9] git-verify-pack: show usage when no pack was specified","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:31Z","receivedAt":"2006-08-10T15:02:31Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n verify-pack.c |    6 ++++++\n 1 files changed, 6 insertions(+), 0 deletions(-)\n\ndiff --git a/verify-pack.c b/verify-pack.c\nindex ef00204..7201596 100644\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -34,6 +34,7 @@ int main(int ac, char **av)\n \tint errs = 0;\n \tint verbose = 0;\n \tint no_more_options = 0;\n+\tint nothing_done = 1;\n \n \twhile (1 < ac) {\n \t\tchar path[PATH_MAX];\n@@ -50,8 +51,13 @@ int main(int ac, char **av)\n \t\t\tstrcpy(path, av[1]);\n \t\t\tif (verify_one_pack(path, verbose))\n \t\t\t\terrs++;\n+\t\t\tnothing_done = 0;\n \t\t}\n \t\tac--; av++;\n \t}\n+\n+\tif (nothing_done)\n+\t\tusage(verify_pack_usage);\n+\n \treturn !!errs;\n }\n-- \n1.4.2.rc2.g822a\n"},{"id":"25097","messageId":"11552221581834-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 3/9] git-verify-pack: more careful path handling","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:32Z","receivedAt":"2006-08-10T15:02:32Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Use strlcpy() to copy the filename into a buffer and complain if it\ndoesn't fit.  Also move the path buffer into verify_one_pack(); it is\nused only there.  Now we can const'ify the first argument of this\nfunction.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n verify-pack.c |   14 ++++++++------\n 1 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/verify-pack.c b/verify-pack.c\nindex 7201596..2c814a6 100644\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -1,10 +1,15 @@\n #include \"cache.h\"\n #include \"pack.h\"\n \n-static int verify_one_pack(char *arg, int verbose)\n+static int verify_one_pack(const char *path, int verbose)\n {\n-\tint len = strlen(arg);\n+\tchar arg[PATH_MAX];\n+\tint len;\n \tstruct packed_git *g;\n+\n+\tlen = strlcpy(arg, path, PATH_MAX);\n+\tif (len >= PATH_MAX)\n+\t\treturn error(\"name too long: %s\", path);\n \t\n \twhile (1) {\n \t\t/* Should name foo.idx, but foo.pack may be named;\n@@ -37,8 +42,6 @@ int main(int ac, char **av)\n \tint nothing_done = 1;\n \n \twhile (1 < ac) {\n-\t\tchar path[PATH_MAX];\n-\n \t\tif (!no_more_options && av[1][0] == '-') {\n \t\t\tif (!strcmp(\"-v\", av[1]))\n \t\t\t\tverbose = 1;\n@@ -48,8 +51,7 @@ int main(int ac, char **av)\n \t\t\t\tusage(verify_pack_usage);\n \t\t}\n \t\telse {\n-\t\t\tstrcpy(path, av[1]);\n-\t\t\tif (verify_one_pack(path, verbose))\n+\t\t\tif (verify_one_pack(av[1], verbose))\n \t\t\t\terrs++;\n \t\t\tnothing_done = 0;\n \t\t}\n-- \n1.4.2.rc2.g822a\n"},{"id":"25098","messageId":"11552221583172-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 4/9] git-verify-pack: insist on .idx extension","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:33Z","receivedAt":"2006-08-10T15:02:33Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"git-verify-pack can be called with a filename without .idx extension.\nadd_packed_git() on the other hand depends on its presence.  So\ninstead of trying to call it with whatever the user gave us check for\nthat extension and add it if it's missing.\n\nThat means that you can't name your index file \"blah\" and your pack\nfile \".pack\" anymore (\"git-verify-pack blah\" currently works in that\ncase).  I think this regression is a good change. ;-)\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n verify-pack.c |   11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff a/verify-pack.c b/verify-pack.c\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -18,13 +18,12 @@\n \t\tif (has_extension(arg, len, \".pack\")) {\n \t\t\tstrcpy(arg + len - 5, \".idx\");\n \t\t\tlen--;\n+\t\t} else if (!has_extension(arg, len, \".idx\")) {\n+\t\t\tif (len + 4 >= PATH_MAX)\n+\t\t\t\treturn error(\"name too long: %s.idx\", arg);\n+\t\t\tstrcpy(arg + len, \".idx\");\n+\t\t\tlen += 4;\n \t\t}\n-\t\t/* Should name foo.idx now */\n-\t\tif ((g = add_packed_git(arg, len, 1)))\n-\t\t\tbreak;\n-\t\t/* No?  did you name just foo? */\n-\t\tstrcpy(arg + len, \".idx\");\n-\t\tlen += 4;\n \t\tif ((g = add_packed_git(arg, len, 1)))\n \t\t\tbreak;\n \t\treturn error(\"packfile %s not found.\", arg);\n"},{"id":"25101","messageId":"11552221581038-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 5/9] git-verify-pack: get rid of while loop","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:34Z","receivedAt":"2006-08-10T15:02:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Get rid of that while loop which was apparently used as a way to avoid\ngoto's (why?).  It's easy now because there is only one break left at\nthe end of it.  Also make the comment clearer.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n verify-pack.c |   32 ++++++++++++++++----------------\n 1 file changed, 16 insertions(+), 16 deletions(-)\n\ndiff a/verify-pack.c b/verify-pack.c\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -11,23 +11,23 @@\n \tif (len >= PATH_MAX)\n \t\treturn error(\"name too long: %s\", path);\n \t\n-\twhile (1) {\n-\t\t/* Should name foo.idx, but foo.pack may be named;\n-\t\t * convert it to foo.idx\n-\t\t */\n-\t\tif (has_extension(arg, len, \".pack\")) {\n-\t\t\tstrcpy(arg + len - 5, \".idx\");\n-\t\t\tlen--;\n-\t\t} else if (!has_extension(arg, len, \".idx\")) {\n-\t\t\tif (len + 4 >= PATH_MAX)\n-\t\t\t\treturn error(\"name too long: %s.idx\", arg);\n-\t\t\tstrcpy(arg + len, \".idx\");\n-\t\t\tlen += 4;\n-\t\t}\n-\t\tif ((g = add_packed_git(arg, len, 1)))\n-\t\t\tbreak;\n-\t\treturn error(\"packfile %s not found.\", arg);\n+\t/*\n+\t * In addition to \"foo.idx\" we accept \"foo.pack\" and \"foo\";\n+\t * normalize these forms to \"foo.idx\" for add_packed_git().\n+\t */\n+\tif (has_extension(arg, len, \".pack\")) {\n+\t\tstrcpy(arg + len - 5, \".idx\");\n+\t\tlen--;\n+\t} else if (!has_extension(arg, len, \".idx\")) {\n+\t\tif (len + 4 >= PATH_MAX)\n+\t\t\treturn error(\"name too long: %s.idx\", arg);\n+\t\tstrcpy(arg + len, \".idx\");\n+\t\tlen += 4;\n \t}\n+\n+\tif (!(g = add_packed_git(arg, len, 1)))\n+\t\treturn error(\"packfile %s not found.\", arg);\n+\n \treturn verify_pack(g, verbose);\n }\n \n"},{"id":"25099","messageId":"11552221582064-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 6/9] git-verify-pack: free pack after use and a cleanup","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:35Z","receivedAt":"2006-08-10T15:02:35Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Plug memory leak in verify_one_pack() by freeing the struct packed_git\nwe got from add_packed_git().  Also rename g to pack and pull an\nassignment out of an if statement while we're at it.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n verify-pack.c |   11 ++++++++---\n 1 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/verify-pack.c b/verify-pack.c\nindex 1dcef52..c7bed1e 100644\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -5,7 +5,8 @@ static int verify_one_pack(const char *p\n {\n \tchar arg[PATH_MAX];\n \tint len;\n-\tstruct packed_git *g;\n+\tstruct packed_git *pack;\n+\tint err;\n \n \tlen = strlcpy(arg, path, PATH_MAX);\n \tif (len >= PATH_MAX)\n@@ -25,10 +26,14 @@ static int verify_one_pack(const char *p\n \t\tlen += 4;\n \t}\n \n-\tif (!(g = add_packed_git(arg, len, 1)))\n+\tpack = add_packed_git(arg, len, 1);\n+\tif (!pack)\n \t\treturn error(\"packfile %s not found.\", arg);\n \n-\treturn verify_pack(g, verbose);\n+\terr = verify_pack(pack, verbose);\n+\tfree(pack);\n+\n+\treturn err;\n }\n \n static const char verify_pack_usage[] = \"git-verify-pack [-v] <pack>...\";\n-- \n1.4.2.rc2.g822a\n"},{"id":"25093","messageId":"11552221583008-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 7/9] git-verify-pack: buffer overrun paranoia","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:36Z","receivedAt":"2006-08-10T15:02:36Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n verify-pack.c |    7 +++++++\n 1 files changed, 7 insertions(+), 0 deletions(-)\n\ndiff --git a/verify-pack.c b/verify-pack.c\nindex 94fe0f3..1076001 100644\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -26,6 +26,15 @@ static int verify_one_pack(const char *p\n \t\tlen += 4;\n \t}\n \n+\t/*\n+\t * add_packed_git() uses our buffer (containing \"foo.idx\") to\n+\t * build the pack filename (\"foo.pack\").  Make sure it fits.\n+\t */\n+\tif (len + 1 >= PATH_MAX) {\n+\t\targ[len - 4] = '\\0';\n+\t\treturn error(\"name too long: %s.pack\", arg);\n+\t}\n+\n \tpack = add_packed_git(arg, len, 1);\n \tif (!pack)\n \t\treturn error(\"packfile %s not found.\", arg);\n-- \n1.4.2.rc2.g822a\n"},{"id":"25102","messageId":"11552221583276-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 8/9] git-verify-pack: no need to count errors","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:37Z","receivedAt":"2006-08-10T15:02:37Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n verify-pack.c |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/verify-pack.c b/verify-pack.c\nindex 1076001..f12cefe 100644\n--- a/verify-pack.c\n+++ b/verify-pack.c\n@@ -49,7 +49,7 @@ static const char verify_pack_usage[] = \n \n int main(int ac, char **av)\n {\n-\tint errs = 0;\n+\tint err = 0;\n \tint verbose = 0;\n \tint no_more_options = 0;\n \tint nothing_done = 1;\n@@ -65,7 +65,7 @@ int main(int ac, char **av)\n \t\t}\n \t\telse {\n \t\t\tif (verify_one_pack(av[1], verbose))\n-\t\t\t\terrs++;\n+\t\t\t\terr = 1;\n \t\t\tnothing_done = 0;\n \t\t}\n \t\tac--; av++;\n@@ -74,5 +74,5 @@ int main(int ac, char **av)\n \tif (nothing_done)\n \t\tusage(verify_pack_usage);\n \n-\treturn !!errs;\n+\treturn err;\n }\n-- \n1.4.2.rc2.g822a\n"},{"id":"25100","messageId":"11552221581436-git-send-email-rene.scharfe@lsrfire.ath.cx","threadId":"5198","inReplyTo":"11552221582769-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"[PATCH 9/9] git-verify-pack: make builtin","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T15:02:38Z","receivedAt":"2006-08-10T15:02:38Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Convert git-verify-pack to a builtin command.  Also rename ac to argc\nand av to argv for consistancy.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n Makefile              |    1 +\n builtin-verify-pack.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++\n builtin.h             |    1 +\n git.c                 |    1 +\n verify-pack.c         |   76 ------------------------------------------------\n 5 files changed, 80 insertions(+), 76 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex faa118a..07c421b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -295,6 +295,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-update-index.o \\\n \tbuiltin-update-ref.o \\\n \tbuiltin-upload-tar.o \\\n+\tbuiltin-verify-pack.o \\\n \tbuiltin-write-tree.o\n \n GITLIBS = $(LIB_FILE) $(XDIFF_LIB)\ndiff --git a/builtin-verify-pack.c b/builtin-verify-pack.c\nnew file mode 100644\nindex 0000000..15f2502\n--- /dev/null\n+++ b/builtin-verify-pack.c\n@@ -0,0 +1,79 @@\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"pack.h\"\n+\n+static int verify_one_pack(const char *path, int verbose)\n+{\n+\tchar arg[PATH_MAX];\n+\tint len;\n+\tstruct packed_git *pack;\n+\tint err;\n+\n+\tlen = strlcpy(arg, path, PATH_MAX);\n+\tif (len >= PATH_MAX)\n+\t\treturn error(\"name too long: %s\", path);\n+\n+\t/*\n+\t * In addition to \"foo.idx\" we accept \"foo.pack\" and \"foo\";\n+\t * normalize these forms to \"foo.idx\" for add_packed_git().\n+\t */\n+\tif (has_extension(arg, len, \".pack\")) {\n+\t\tstrcpy(arg + len - 5, \".idx\");\n+\t\tlen--;\n+\t} else if (!has_extension(arg, len, \".idx\")) {\n+\t\tif (len + 4 >= PATH_MAX)\n+\t\t\treturn error(\"name too long: %s.idx\", arg);\n+\t\tstrcpy(arg + len, \".idx\");\n+\t\tlen += 4;\n+\t}\n+\n+\t/*\n+\t * add_packed_git() uses our buffer (containing \"foo.idx\") to\n+\t * build the pack filename (\"foo.pack\").  Make sure it fits.\n+\t */\n+\tif (len + 1 >= PATH_MAX) {\n+\t\targ[len - 4] = '\\0';\n+\t\treturn error(\"name too long: %s.pack\", arg);\n+\t}\n+\n+\tpack = add_packed_git(arg, len, 1);\n+\tif (!pack)\n+\t\treturn error(\"packfile %s not found.\", arg);\n+\n+\terr = verify_pack(pack, verbose);\n+\tfree(pack);\n+\n+\treturn err;\n+}\n+\n+static const char verify_pack_usage[] = \"git-verify-pack [-v] <pack>...\";\n+\n+int cmd_verify_pack(int argc, const char **argv, const char *prefix)\n+{\n+\tint err = 0;\n+\tint verbose = 0;\n+\tint no_more_options = 0;\n+\tint nothing_done = 1;\n+\n+\twhile (1 < argc) {\n+\t\tif (!no_more_options && argv[1][0] == '-') {\n+\t\t\tif (!strcmp(\"-v\", argv[1]))\n+\t\t\t\tverbose = 1;\n+\t\t\telse if (!strcmp(\"--\", argv[1]))\n+\t\t\t\tno_more_options = 1;\n+\t\t\telse\n+\t\t\t\tusage(verify_pack_usage);\n+\t\t}\n+\t\telse {\n+\t\t\tif (verify_one_pack(argv[1], verbose))\n+\t\t\t\terr = 1;\n+\t\t\tnothing_done = 0;\n+\t\t}\n+\t\targc--; argv++;\n+\t}\n+\n+\tif (nothing_done)\n+\t\tusage(verify_pack_usage);\n+\n+\treturn err;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex c0bdb05..ade58c4 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -59,5 +59,6 @@ extern int cmd_upload_tar(int argc, cons\n extern int cmd_version(int argc, const char **argv, const char *prefix);\n extern int cmd_whatchanged(int argc, const char **argv, const char *prefix);\n extern int cmd_write_tree(int argc, const char **argv, const char *prefix);\n+extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);\n \n #endif\ndiff --git a/git.c b/git.c\nindex db0f867..5da7787 100644\n--- a/git.c\n+++ b/git.c\n@@ -270,6 +270,7 @@ static void handle_internal_command(int \n \t\t{ \"version\", cmd_version },\n \t\t{ \"whatchanged\", cmd_whatchanged, RUN_SETUP | USE_PAGER },\n \t\t{ \"write-tree\", cmd_write_tree, RUN_SETUP },\n+\t\t{ \"verify-pack\", cmd_verify_pack },\n \t};\n \tint i;\n \ndiff --git a/verify-pack.c b/verify-pack.c\ndeleted file mode 100644\nindex f12cefe..0000000\n--- a/verify-pack.c\n+++ /dev/null\n@@ -1,78 +0,0 @@\n-#include \"cache.h\"\n-#include \"pack.h\"\n-\n-static int verify_one_pack(const char *path, int verbose)\n-{\n-\tchar arg[PATH_MAX];\n-\tint len;\n-\tstruct packed_git *pack;\n-\tint err;\n-\n-\tlen = strlcpy(arg, path, PATH_MAX);\n-\tif (len >= PATH_MAX)\n-\t\treturn error(\"name too long: %s\", path);\n-\t\n-\t/*\n-\t * In addition to \"foo.idx\" we accept \"foo.pack\" and \"foo\";\n-\t * normalize these forms to \"foo.idx\" for add_packed_git().\n-\t */\n-\tif (has_extension(arg, len, \".pack\")) {\n-\t\tstrcpy(arg + len - 5, \".idx\");\n-\t\tlen--;\n-\t} else if (!has_extension(arg, len, \".idx\")) {\n-\t\tif (len + 4 >= PATH_MAX)\n-\t\t\treturn error(\"name too long: %s.idx\", arg);\n-\t\tstrcpy(arg + len, \".idx\");\n-\t\tlen += 4;\n-\t}\n-\n-\t/*\n-\t * add_packed_git() uses our buffer (containing \"foo.idx\") to\n-\t * build the pack filename (\"foo.pack\").  Make sure it fits.\n-\t */\n-\tif (len + 1 >= PATH_MAX) {\n-\t\targ[len - 4] = '\\0';\n-\t\treturn error(\"name too long: %s.pack\", arg);\n-\t}\n-\n-\tpack = add_packed_git(arg, len, 1);\n-\tif (!pack)\n-\t\treturn error(\"packfile %s not found.\", arg);\n-\n-\terr = verify_pack(pack, verbose);\n-\tfree(pack);\n-\n-\treturn err;\n-}\n-\n-static const char verify_pack_usage[] = \"git-verify-pack [-v] <pack>...\";\n-\n-int main(int ac, char **av)\n-{\n-\tint err = 0;\n-\tint verbose = 0;\n-\tint no_more_options = 0;\n-\tint nothing_done = 1;\n-\n-\twhile (1 < ac) {\n-\t\tif (!no_more_options && av[1][0] == '-') {\n-\t\t\tif (!strcmp(\"-v\", av[1]))\n-\t\t\t\tverbose = 1;\n-\t\t\telse if (!strcmp(\"--\", av[1]))\n-\t\t\t\tno_more_options = 1;\n-\t\t\telse\n-\t\t\t\tusage(verify_pack_usage);\n-\t\t}\n-\t\telse {\n-\t\t\tif (verify_one_pack(av[1], verbose))\n-\t\t\t\terr = 1;\n-\t\t\tnothing_done = 0;\n-\t\t}\n-\t\tac--; av++;\n-\t}\n-\n-\tif (nothing_done)\n-\t\tusage(verify_pack_usage);\n-\n-\treturn err;\n-}\n-- \n1.4.2.rc2.g822a\n"},{"id":"25104","messageId":"Pine.LNX.4.63.0608101822550.10541@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5198","inReplyTo":"11552221583276-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"Re: [PATCH 8/9] git-verify-pack: no need to count errors","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-08-10T16:23:27Z","receivedAt":"2006-08-10T16:23:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nthere was no harm done counting the errors, i.e. no need to \"fix\" that.\n\nCiao,\nDscho\n"},{"id":"25105","messageId":"Pine.LNX.4.63.0608101823500.10541@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5198","inReplyTo":"11552221581436-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"Re: [PATCH 9/9] git-verify-pack: make builtin","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-08-10T16:24:50Z","receivedAt":"2006-08-10T16:24:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 10 Aug 2006, Rene Scharfe wrote:\n\n>  Makefile              |    1 +\n>  builtin-verify-pack.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++++\n>  builtin.h             |    1 +\n>  git.c                 |    1 +\n>  verify-pack.c         |   76 ------------------------------------------------\n>  5 files changed, 80 insertions(+), 76 deletions(-)\n\nThis would look much nicer with \"-M\". Maybe we should make that a default \nfor git-format-patch?\n\nCiao,\nDscho\n"},{"id":"25106","messageId":"Pine.LNX.4.63.0608101825000.10541@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"5198","inReplyTo":"11552221581171-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"Re: [PATCH 1/9] Add has_extension()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-08-10T16:25:43Z","receivedAt":"2006-08-10T16:25:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 10 Aug 2006, Rene Scharfe wrote:\n\n> The little helper has_extension() documents through its name what we are\n> trying to do and makes sure we don't forget the underrun check.\n\nWhile I think it is a good change, it is independent of verify-pack, no?\n\nCiao,\nDscho\n"},{"id":"25108","messageId":"44DB6ECE.10706@lsrfire.ath.cx","threadId":"5198","inReplyTo":"Pine.LNX.4.63.0608101822550.10541@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 8/9] git-verify-pack: no need to count errors","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T17:37:18Z","receivedAt":"2006-08-10T17:37:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Schindelin schrieb:\n> there was no harm done counting the errors, i.e. no need to \"fix\" that.\n\nYes, it's more of a style thing.  Why count when you don't need the\nnumber?  Why use \"!!\" when you can use \"1\" explicitly?\n\nAnd then there's the danger of counter wrap-around. ;-)\n\nIt's surely one of those bikeshed colour issues, admittedly.\n\nThanks,\nRené\n"},{"id":"25109","messageId":"44DB7178.4090706@lsrfire.ath.cx","threadId":"5198","inReplyTo":"Pine.LNX.4.63.0608101825000.10541@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 1/9] Add has_extension()","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T17:48:40Z","receivedAt":"2006-08-10T17:48:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Schindelin schrieb:\n> On Thu, 10 Aug 2006, Rene Scharfe wrote:\n> \n>> The little helper has_extension() documents through its name what we are\n>> trying to do and makes sure we don't forget the underrun check.\n> \n> While I think it is a good change, it is independent of verify-pack, no?\n\nI could have limited has_extension() to verify-pack.c initially and then\nsent a conversion for the other files after this series had been\naccepted, yes.  It's one of the main cleanups for verify_one_pack(),\nthough, because it allowed me to drop some comments -- without losing\nreadability (IMHO).\n\nI could have documented it broader scope better in the commit message. :-/\n\nThanks,\nRené\n"},{"id":"25110","messageId":"20060810182117.GA13446@c165.ib.student.liu.se","threadId":"5198","inReplyTo":"11552221581171-git-send-email-rene.scharfe@lsrfire.ath.cx","subject":"Re: [PATCH 1/9] Add has_extension()","fromName":"Fredrik Kuivinen","fromEmail":"freku045@student.liu.se","sentAt":"2006-08-10T18:21:18Z","receivedAt":"2006-08-10T18:21:18Z","isPatch":true,"sender":{"key":"frekui@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13770967?v=4"},"body":"On Thu, Aug 10, 2006 at 05:02:30PM +0200, Rene Scharfe wrote:\n> +static inline int has_extension(const char *filename, int len, const char *ext)\n> +{\n> +\tint extlen = strlen(ext);\n> +\treturn len > extlen && !memcmp(filename + len - extlen, ext, extlen);\n> +}\n> +\n\nWouldn't this function be much easier to use if len is computed from\nfilename with strlen? (after a quick look through the other patches I\ncouldn't find a call site where filename wasn't NUL-terminated)\n\n- Fredrik\n"},{"id":"25111","messageId":"44DB7E01.4000901@lsrfire.ath.cx","threadId":"5198","inReplyTo":"20060810182117.GA13446@c165.ib.student.liu.se","subject":"Re: [PATCH 1/9] Add has_extension()","fromName":"Rene Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2006-08-10T18:42:09Z","receivedAt":"2006-08-10T18:42:09Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Fredrik Kuivinen schrieb:\n> On Thu, Aug 10, 2006 at 05:02:30PM +0200, Rene Scharfe wrote:\n>> +static inline int has_extension(const char *filename, int len,\n>> const char *ext) +{ +\tint extlen = strlen(ext); +\treturn len >\n>> extlen && !memcmp(filename + len - extlen, ext, extlen); +} +\n> \n> Wouldn't this function be much easier to use if len is computed from \n> filename with strlen? (after a quick look through the other patches I\n>  couldn't find a call site where filename wasn't NUL-terminated)\n\nYes, it would be a bit easier, and my first version had only two\narguments.  Then I found out that the length of the first string is\nalready known at _all_ potential callsites, using this command to\nidentify candidates:\n\n\t$ grep 'cmp.*\"\\..*\"' *.[ch]\n\nWe could add something like this:\n\n\t#define has_ext(a, b) has_extension(a, strlen(a), b)\n\nto make it easier to use for code that doesn't already determine the\nstring length.  I think we should add it only after a user has been\nidentified, though.\n\nThanks,\nRené\n"},{"id":"25119","messageId":"20060810214718.GD13446@c165.ib.student.liu.se","threadId":"5198","inReplyTo":"44DB7E01.4000901@lsrfire.ath.cx","subject":"Re: [PATCH 1/9] Add has_extension()","fromName":"Fredrik Kuivinen","fromEmail":"freku045@student.liu.se","sentAt":"2006-08-10T21:47:18Z","receivedAt":"2006-08-10T21:47:18Z","isPatch":true,"sender":{"key":"frekui@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13770967?v=4"},"body":"On Thu, Aug 10, 2006 at 08:42:09PM +0200, Rene Scharfe wrote:\n> Fredrik Kuivinen schrieb:\n> > On Thu, Aug 10, 2006 at 05:02:30PM +0200, Rene Scharfe wrote:\n> >> +static inline int has_extension(const char *filename, int len,\n> >> const char *ext) +{ +\tint extlen = strlen(ext); +\treturn len >\n> >> extlen && !memcmp(filename + len - extlen, ext, extlen); +} +\n> > \n> > Wouldn't this function be much easier to use if len is computed from \n> > filename with strlen? (after a quick look through the other patches I\n> >  couldn't find a call site where filename wasn't NUL-terminated)\n> \n> Yes, it would be a bit easier, and my first version had only two\n> arguments.  Then I found out that the length of the first string is\n> already known at _all_ potential callsites, using this command to\n> identify candidates:\n> \n> \t$ grep 'cmp.*\"\\..*\"' *.[ch]\n> \n> We could add something like this:\n> \n> \t#define has_ext(a, b) has_extension(a, strlen(a), b)\n> \n> to make it easier to use for code that doesn't already determine the\n> string length.  I think we should add it only after a user has been\n> identified, though.\n> \n\nIMHO the small speed-up isn't worth it. Drop the extra argument and\navoid a possible future bug.\n\n- Fredrik\n"}]}