{"thread":{"id":"30857","subject":"[PATCH v2 1/9] sha1_name.c: indentation fix","startedAt":"2012-06-21T06:35:23Z","lastAt":"2012-06-21T19:57:19Z","messageCount":12,"participants":["Junio C Hamano","Erik Faye-Lund"],"isPatch":true,"patchVersion":2,"patchTotal":9},"messages":[{"id":"194021","messageId":"1340260532-11378-1-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":null,"subject":"[PATCH v2 0/9] Extending the shelf-life of \"git describe\" output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:23Z","receivedAt":"2012-06-21T06:35:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is take 2.  The first round was $gmane/200165.\n\nA major difference from v1 is the [PATCH 6/9].  The earlier approach\nwas to filter out candidates that match the given prefix for\nadditional criteria as they are found in the loop, but this round\ntries to optimize for the common case of not having ambiguities.  We\npostpone running additional test until we find the second object\nthat match the prefix (in other words, if there is only one object\nthat has the prefix, we do not apply the \"we know this name refers\nto a commit\" hint, and let the caller deal with a non commit object,\njust like the current code does).\n\nJunio C Hamano (9):\n  sha1_name.c: indentation fix\n  sha1_name.c: clarify what \"fake\" is for in find_short_object_filename()\n  sha1_name.c: rename \"now\" to \"current\"\n  sha1_name.c: refactor find_short_packed_object()\n  sha1_name.c: correct misnamed \"canonical\" and \"res\"\n  sha1_name.c: restructure disambiguation of short names\n  sha1_name.c: allow get_short_sha1() to take other flags\n  sha1_name.c: teach get_short_sha1() a commit-only option\n  sha1_name.c: get_describe_name() by definition groks only commits\n\n sha1_name.c | 282 +++++++++++++++++++++++++++++++++++++++---------------------\n 1 file changed, 182 insertions(+), 100 deletions(-)\n\n-- \n1.7.11.2.gd284367\n"},{"id":"194020","messageId":"1340260532-11378-2-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 1/9] sha1_name.c: indentation fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:24Z","receivedAt":"2012-06-21T06:35:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 03ffc2c..5b0c845 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -103,10 +103,10 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \t\t}\n \t\tif (first < num) {\n \t\t\tconst unsigned char *now, *next;\n-\t\t       now = nth_packed_object_sha1(p, first);\n+\t\t\tnow = nth_packed_object_sha1(p, first);\n \t\t\tif (match_sha(len, match, now)) {\n \t\t\t\tnext = nth_packed_object_sha1(p, first+1);\n-\t\t\t       if (!next|| !match_sha(len, match, next)) {\n+\t\t\t\tif (!next|| !match_sha(len, match, next)) {\n \t\t\t\t\t/* unique within this pack */\n \t\t\t\t\tif (!found) {\n \t\t\t\t\t\tfound_sha1 = now;\n-- \n1.7.11.2.gd284367\n"},{"id":"194023","messageId":"1340260532-11378-3-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 2/9] sha1_name.c: clarify what \"fake\" is for in find_short_object_filename()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:25Z","receivedAt":"2012-06-21T06:35:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 5b0c845..4cbca34 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -17,6 +17,13 @@ static int find_short_object_filename(int len, const char *name, unsigned char *\n \tstatic struct alternate_object_database *fakeent;\n \n \tif (!fakeent) {\n+\t\t/*\n+\t\t * Create a \"fake\" alternate object database that\n+\t\t * points to our own object database, to make it\n+\t\t * easier to get a temporary working space in\n+\t\t * alt->name/alt->base while iterating over the\n+\t\t * object databases including our own.\n+\t\t */\n \t\tconst char *objdir = get_object_directory();\n \t\tint objdir_len = strlen(objdir);\n \t\tint entlen = objdir_len + 43;\n-- \n1.7.11.2.gd284367\n"},{"id":"194027","messageId":"1340260532-11378-4-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 3/9] sha1_name.c: rename \"now\" to \"current\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:26Z","receivedAt":"2012-06-21T06:35:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This variable points at the element we are currently looking at, and\ndoes not have anything to do with the current time which the name\n\"now\" implies.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 4cbca34..5224f39 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -93,11 +93,11 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \t\tlast = num;\n \t\twhile (first < last) {\n \t\t\tuint32_t mid = (first + last) / 2;\n-\t\t\tconst unsigned char *now;\n+\t\t\tconst unsigned char *current;\n \t\t\tint cmp;\n \n-\t\t\tnow = nth_packed_object_sha1(p, mid);\n-\t\t\tcmp = hashcmp(match, now);\n+\t\t\tcurrent = nth_packed_object_sha1(p, mid);\n+\t\t\tcmp = hashcmp(match, current);\n \t\t\tif (!cmp) {\n \t\t\t\tfirst = mid;\n \t\t\t\tbreak;\n@@ -109,17 +109,17 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \t\t\tlast = mid;\n \t\t}\n \t\tif (first < num) {\n-\t\t\tconst unsigned char *now, *next;\n-\t\t\tnow = nth_packed_object_sha1(p, first);\n-\t\t\tif (match_sha(len, match, now)) {\n+\t\t\tconst unsigned char *current, *next;\n+\t\t\tcurrent = nth_packed_object_sha1(p, first);\n+\t\t\tif (match_sha(len, match, current)) {\n \t\t\t\tnext = nth_packed_object_sha1(p, first+1);\n \t\t\t\tif (!next|| !match_sha(len, match, next)) {\n \t\t\t\t\t/* unique within this pack */\n \t\t\t\t\tif (!found) {\n-\t\t\t\t\t\tfound_sha1 = now;\n+\t\t\t\t\t\tfound_sha1 = current;\n \t\t\t\t\t\tfound++;\n \t\t\t\t\t}\n-\t\t\t\t\telse if (hashcmp(found_sha1, now)) {\n+\t\t\t\t\telse if (hashcmp(found_sha1, current)) {\n \t\t\t\t\t\tfound = 2;\n \t\t\t\t\t\tbreak;\n \t\t\t\t\t}\n-- \n1.7.11.2.gd284367\n"},{"id":"194024","messageId":"1340260532-11378-5-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 4/9] sha1_name.c: refactor find_short_packed_object()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:27Z","receivedAt":"2012-06-21T06:35:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Extract the logic to find object(s) that match a given prefix inside\na single pack into a separate helper function, and give it a bit more\ncomment.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 103 +++++++++++++++++++++++++++++++++---------------------------\n 1 file changed, 56 insertions(+), 47 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 5224f39..e03992c 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -78,6 +78,59 @@ static int match_sha(unsigned len, const unsigned char *a, const unsigned char *\n \treturn 1;\n }\n \n+static int unique_in_pack(int len,\n+\t\t\t  const unsigned char *match,\n+\t\t\t  struct packed_git *p,\n+\t\t\t  const unsigned char **found_sha1,\n+\t\t\t  int seen_so_far)\n+{\n+\tuint32_t num, last, i, first = 0;\n+\tconst unsigned char *current = NULL;\n+\n+\topen_pack_index(p);\n+\tnum = p->num_objects;\n+\tlast = num;\n+\twhile (first < last) {\n+\t\tuint32_t mid = (first + last) / 2;\n+\t\tconst unsigned char *current;\n+\t\tint cmp;\n+\n+\t\tcurrent = nth_packed_object_sha1(p, mid);\n+\t\tcmp = hashcmp(match, current);\n+\t\tif (!cmp) {\n+\t\t\tfirst = mid;\n+\t\t\tbreak;\n+\t\t}\n+\t\tif (cmp > 0) {\n+\t\t\tfirst = mid+1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tlast = mid;\n+\t}\n+\n+\t/*\n+\t * At this point, \"first\" is the location of the lowest object\n+\t * with an object name that could match \"match\".  See if we have\n+\t * 0, 1 or more objects that actually match(es).\n+\t */\n+\tfor (i = first; i < num; i++) {\n+\t\tcurrent = nth_packed_object_sha1(p, first);\n+\t\tif (!match_sha(len, match, current))\n+\t\t\tbreak;\n+\n+\t\t/* current matches */\n+\t\tif (!seen_so_far) {\n+\t\t\t*found_sha1 = current;\n+\t\t\tseen_so_far++;\n+\t\t} else if (seen_so_far) {\n+\t\t\t/* is it the same as the one previously found elsewhere? */\n+\t\t\tif (hashcmp(*found_sha1, current))\n+\t\t\t\treturn 2; /* definitely not unique */\n+\t\t}\n+\t}\n+\treturn seen_so_far;\n+}\n+\n static int find_short_packed_object(int len, const unsigned char *match, unsigned char *sha1)\n {\n \tstruct packed_git *p;\n@@ -85,53 +138,9 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \tint found = 0;\n \n \tprepare_packed_git();\n-\tfor (p = packed_git; p && found < 2; p = p->next) {\n-\t\tuint32_t num, last;\n-\t\tuint32_t first = 0;\n-\t\topen_pack_index(p);\n-\t\tnum = p->num_objects;\n-\t\tlast = num;\n-\t\twhile (first < last) {\n-\t\t\tuint32_t mid = (first + last) / 2;\n-\t\t\tconst unsigned char *current;\n-\t\t\tint cmp;\n-\n-\t\t\tcurrent = nth_packed_object_sha1(p, mid);\n-\t\t\tcmp = hashcmp(match, current);\n-\t\t\tif (!cmp) {\n-\t\t\t\tfirst = mid;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t\tif (cmp > 0) {\n-\t\t\t\tfirst = mid+1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t\tlast = mid;\n-\t\t}\n-\t\tif (first < num) {\n-\t\t\tconst unsigned char *current, *next;\n-\t\t\tcurrent = nth_packed_object_sha1(p, first);\n-\t\t\tif (match_sha(len, match, current)) {\n-\t\t\t\tnext = nth_packed_object_sha1(p, first+1);\n-\t\t\t\tif (!next|| !match_sha(len, match, next)) {\n-\t\t\t\t\t/* unique within this pack */\n-\t\t\t\t\tif (!found) {\n-\t\t\t\t\t\tfound_sha1 = current;\n-\t\t\t\t\t\tfound++;\n-\t\t\t\t\t}\n-\t\t\t\t\telse if (hashcmp(found_sha1, current)) {\n-\t\t\t\t\t\tfound = 2;\n-\t\t\t\t\t\tbreak;\n-\t\t\t\t\t}\n-\t\t\t\t}\n-\t\t\t\telse {\n-\t\t\t\t\t/* not even unique within this pack */\n-\t\t\t\t\tfound = 2;\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t}\n-\t}\n+\tfor (p = packed_git; p && found < 2; p = p->next)\n+\t\tfound = unique_in_pack(len, match, p, &found_sha1, found);\n+\n \tif (found == 1)\n \t\thashcpy(sha1, found_sha1);\n \treturn found;\n-- \n1.7.11.2.gd284367\n"},{"id":"194025","messageId":"1340260532-11378-6-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 5/9] sha1_name.c: correct misnamed \"canonical\" and \"res\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:28Z","receivedAt":"2012-06-21T06:35:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"These are hexadecimal and binary representation of the short object\nname given to the callchain as its input.  Rename them with _pfx\nsuffix to make it clear they are prefixes, and call them hex and bin\nrespectively.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 44 ++++++++++++++++++++++----------------------\n 1 file changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex e03992c..7062f72 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -9,7 +9,7 @@\n \n static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);\n \n-static int find_short_object_filename(int len, const char *name, unsigned char *sha1)\n+static int find_short_object_filename(int len, const char *hex_pfx, unsigned char *sha1)\n {\n \tstruct alternate_object_database *alt;\n \tchar hex[40];\n@@ -34,18 +34,18 @@ static int find_short_object_filename(int len, const char *name, unsigned char *\n \t}\n \tfakeent->next = alt_odb_list;\n \n-\tsprintf(hex, \"%.2s\", name);\n+\tsprintf(hex, \"%.2s\", hex_pfx);\n \tfor (alt = fakeent; alt && found < 2; alt = alt->next) {\n \t\tstruct dirent *de;\n \t\tDIR *dir;\n-\t\tsprintf(alt->name, \"%.2s/\", name);\n+\t\tsprintf(alt->name, \"%.2s/\", hex_pfx);\n \t\tdir = opendir(alt->base);\n \t\tif (!dir)\n \t\t\tcontinue;\n \t\twhile ((de = readdir(dir)) != NULL) {\n \t\t\tif (strlen(de->d_name) != 38)\n \t\t\t\tcontinue;\n-\t\t\tif (memcmp(de->d_name, name + 2, len - 2))\n+\t\t\tif (memcmp(de->d_name, hex_pfx + 2, len - 2))\n \t\t\t\tcontinue;\n \t\t\tif (!found) {\n \t\t\t\tmemcpy(hex + 2, de->d_name, 38);\n@@ -79,7 +79,7 @@ static int match_sha(unsigned len, const unsigned char *a, const unsigned char *\n }\n \n static int unique_in_pack(int len,\n-\t\t\t  const unsigned char *match,\n+\t\t\t  const unsigned char *bin_pfx,\n \t\t\t  struct packed_git *p,\n \t\t\t  const unsigned char **found_sha1,\n \t\t\t  int seen_so_far)\n@@ -96,7 +96,7 @@ static int unique_in_pack(int len,\n \t\tint cmp;\n \n \t\tcurrent = nth_packed_object_sha1(p, mid);\n-\t\tcmp = hashcmp(match, current);\n+\t\tcmp = hashcmp(bin_pfx, current);\n \t\tif (!cmp) {\n \t\t\tfirst = mid;\n \t\t\tbreak;\n@@ -110,12 +110,12 @@ static int unique_in_pack(int len,\n \n \t/*\n \t * At this point, \"first\" is the location of the lowest object\n-\t * with an object name that could match \"match\".  See if we have\n+\t * with an object name that could match \"bin_pfx\".  See if we have\n \t * 0, 1 or more objects that actually match(es).\n \t */\n \tfor (i = first; i < num; i++) {\n \t\tcurrent = nth_packed_object_sha1(p, first);\n-\t\tif (!match_sha(len, match, current))\n+\t\tif (!match_sha(len, bin_pfx, current))\n \t\t\tbreak;\n \n \t\t/* current matches */\n@@ -131,7 +131,7 @@ static int unique_in_pack(int len,\n \treturn seen_so_far;\n }\n \n-static int find_short_packed_object(int len, const unsigned char *match, unsigned char *sha1)\n+static int find_short_packed_object(int len, const unsigned char *bin_pfx, unsigned char *sha1)\n {\n \tstruct packed_git *p;\n \tconst unsigned char *found_sha1 = NULL;\n@@ -139,7 +139,7 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n \n \tprepare_packed_git();\n \tfor (p = packed_git; p && found < 2; p = p->next)\n-\t\tfound = unique_in_pack(len, match, p, &found_sha1, found);\n+\t\tfound = unique_in_pack(len, bin_pfx, p, &found_sha1, found);\n \n \tif (found == 1)\n \t\thashcpy(sha1, found_sha1);\n@@ -149,15 +149,15 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n #define SHORT_NAME_NOT_FOUND (-1)\n #define SHORT_NAME_AMBIGUOUS (-2)\n \n-static int find_unique_short_object(int len, char *canonical,\n-\t\t\t\t    unsigned char *res, unsigned char *sha1)\n+static int find_unique_short_object(int len, char *hex_pfx,\n+\t\t\t\t    unsigned char *bin_pfx, unsigned char *sha1)\n {\n \tint has_unpacked, has_packed;\n \tunsigned char unpacked_sha1[20], packed_sha1[20];\n \n \tprepare_alt_odb();\n-\thas_unpacked = find_short_object_filename(len, canonical, unpacked_sha1);\n-\thas_packed = find_short_packed_object(len, res, packed_sha1);\n+\thas_unpacked = find_short_object_filename(len, hex_pfx, unpacked_sha1);\n+\thas_packed = find_short_packed_object(len, bin_pfx, packed_sha1);\n \tif (!has_unpacked && !has_packed)\n \t\treturn SHORT_NAME_NOT_FOUND;\n \tif (1 < has_unpacked || 1 < has_packed)\n@@ -177,13 +177,13 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \t\t\t  int quietly)\n {\n \tint i, status;\n-\tchar canonical[40];\n-\tunsigned char res[20];\n+\tchar hex_pfx[40];\n+\tunsigned char bin_pfx[20];\n \n \tif (len < MINIMUM_ABBREV || len > 40)\n \t\treturn -1;\n-\thashclr(res);\n-\tmemset(canonical, 'x', 40);\n+\thashclr(bin_pfx);\n+\tmemset(hex_pfx, 'x', 40);\n \tfor (i = 0; i < len ;i++) {\n \t\tunsigned char c = name[i];\n \t\tunsigned char val;\n@@ -197,15 +197,15 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \t\t}\n \t\telse\n \t\t\treturn -1;\n-\t\tcanonical[i] = c;\n+\t\thex_pfx[i] = c;\n \t\tif (!(i & 1))\n \t\t\tval <<= 4;\n-\t\tres[i >> 1] |= val;\n+\t\tbin_pfx[i >> 1] |= val;\n \t}\n \n-\tstatus = find_unique_short_object(i, canonical, res, sha1);\n+\tstatus = find_unique_short_object(i, hex_pfx, bin_pfx, sha1);\n \tif (!quietly && (status == SHORT_NAME_AMBIGUOUS))\n-\t\treturn error(\"short SHA1 %.*s is ambiguous.\", len, canonical);\n+\t\treturn error(\"short SHA1 %.*s is ambiguous.\", len, hex_pfx);\n \treturn status;\n }\n \n-- \n1.7.11.2.gd284367\n"},{"id":"194029","messageId":"1340260532-11378-7-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 6/9] sha1_name.c: restructure disambiguation of short names","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:29Z","receivedAt":"2012-06-21T06:35:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We try to find zero, one or more matches from loose objects and\npacked objects independently and then decide if the given short\nobject name is unique across them.\n\nInstead, introduce a \"struct disambiguate_state\" that keeps track of\nwhat we have found so far, that can be one of:\n\n - We have seen one object that _could_ be what we are looking for;\n - We have also checked that object for additional constraints (if any),\n   and found that the object satisfies it;\n - We have also checked that object for additional constraints (if any),\n   and found that the object does not satisfy it; or\n - We have seen more than one objects that satisfy the constraints.\n\nand pass it to the enumeration functions for loose and packed\nobjects.  The disambiguation state can optionally take a callback\nfunction that takes a candidate object name and reports if the\nobject satisifies additional criteria (e.g. when the caller knows\nthat the short name must refer to a commit, this mechanism can be\nused to check the type of the given object).\n\nCompared to the earlier attempt, this round avoids the optional\ncheck if there is only one candidate that matches the short name in\nthe first place.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 172 +++++++++++++++++++++++++++++++++++++++---------------------\n 1 file changed, 112 insertions(+), 60 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 7062f72..852d5c2 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -9,11 +9,67 @@\n \n static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);\n \n-static int find_short_object_filename(int len, const char *hex_pfx, unsigned char *sha1)\n+typedef int (*disambiguate_hint_fn)(const unsigned char *, void *);\n+\n+struct disambiguate_state {\n+\tdisambiguate_hint_fn fn;\n+\tvoid *cb_data;\n+\tunsigned char candidate[20];\n+\tunsigned candidate_exists:1;\n+\tunsigned candidate_checked:1;\n+\tunsigned candidate_ok:1;\n+\tunsigned disambiguate_fn_used:1;\n+\tunsigned ambiguous:1;\n+};\n+\n+static void update_candidates(struct disambiguate_state *ds, const unsigned char *current)\n+{\n+\tif (!ds->candidate_exists) {\n+\t\t/* this is the first candidate */\n+\t\thashcpy(ds->candidate, current);\n+\t\tds->candidate_exists = 1;\n+\t\treturn;\n+\t} else if (!hashcmp(ds->candidate, current)) {\n+\t\t/* the same as what we already have seen */\n+\t\treturn;\n+\t}\n+\n+\tif (!ds->fn) {\n+\t\t/* cannot disambiguate between ds->candidate and current */\n+\t\tds->ambiguous = 1;\n+\t\treturn;\n+\t}\n+\n+\tif (!ds->candidate_checked) {\n+\t\tds->candidate_ok = ds->fn(ds->candidate, ds->cb_data);\n+\t\tds->disambiguate_fn_used = 1;\n+\t\tds->candidate_checked = 1;\n+\t}\n+\n+\tif (!ds->candidate_ok) {\n+\t\t/* discard the candidate; we know it does not satisify fn */\n+\t\thashcpy(ds->candidate, current);\n+\t\tds->candidate_checked = 0;\n+\t\treturn;\n+\t}\n+\n+\t/* if we reach this point, we know ds->candidate satisfies fn */\n+\tif (ds->fn(current, ds->cb_data)) {\n+\t\t/*\n+\t\t * if both current and candidate satisfy fn, we cannot\n+\t\t * disambiguate.\n+\t\t */\n+\t\tds->candidate_ok = 0;\n+\t\tds->ambiguous = 1;\n+\t}\n+\n+\t/* otherwise, current can be discarded and candidate is still good */\n+}\n+\n+static void find_short_object_filename(int len, const char *hex_pfx, struct disambiguate_state *ds)\n {\n \tstruct alternate_object_database *alt;\n \tchar hex[40];\n-\tint found = 0;\n \tstatic struct alternate_object_database *fakeent;\n \n \tif (!fakeent) {\n@@ -35,32 +91,27 @@ static int find_short_object_filename(int len, const char *hex_pfx, unsigned cha\n \tfakeent->next = alt_odb_list;\n \n \tsprintf(hex, \"%.2s\", hex_pfx);\n-\tfor (alt = fakeent; alt && found < 2; alt = alt->next) {\n+\tfor (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {\n \t\tstruct dirent *de;\n \t\tDIR *dir;\n \t\tsprintf(alt->name, \"%.2s/\", hex_pfx);\n \t\tdir = opendir(alt->base);\n \t\tif (!dir)\n \t\t\tcontinue;\n-\t\twhile ((de = readdir(dir)) != NULL) {\n+\n+\t\twhile (!ds->ambiguous && (de = readdir(dir)) != NULL) {\n+\t\t\tunsigned char sha1[20];\n+\n \t\t\tif (strlen(de->d_name) != 38)\n \t\t\t\tcontinue;\n \t\t\tif (memcmp(de->d_name, hex_pfx + 2, len - 2))\n \t\t\t\tcontinue;\n-\t\t\tif (!found) {\n-\t\t\t\tmemcpy(hex + 2, de->d_name, 38);\n-\t\t\t\tfound++;\n-\t\t\t}\n-\t\t\telse if (memcmp(hex + 2, de->d_name, 38)) {\n-\t\t\t\tfound = 2;\n-\t\t\t\tbreak;\n-\t\t\t}\n+\t\t\tmemcpy(hex + 2, de->d_name, 38);\n+\t\t\tif (!get_sha1_hex(hex, sha1))\n+\t\t\t\tupdate_candidates(ds, sha1);\n \t\t}\n \t\tclosedir(dir);\n \t}\n-\tif (found == 1)\n-\t\treturn get_sha1_hex(hex, sha1) == 0;\n-\treturn found;\n }\n \n static int match_sha(unsigned len, const unsigned char *a, const unsigned char *b)\n@@ -78,11 +129,10 @@ static int match_sha(unsigned len, const unsigned char *a, const unsigned char *\n \treturn 1;\n }\n \n-static int unique_in_pack(int len,\n+static void unique_in_pack(int len,\n \t\t\t  const unsigned char *bin_pfx,\n-\t\t\t  struct packed_git *p,\n-\t\t\t  const unsigned char **found_sha1,\n-\t\t\t  int seen_so_far)\n+\t\t\t   struct packed_git *p,\n+\t\t\t   struct disambiguate_state *ds)\n {\n \tuint32_t num, last, i, first = 0;\n \tconst unsigned char *current = NULL;\n@@ -113,63 +163,58 @@ static int unique_in_pack(int len,\n \t * with an object name that could match \"bin_pfx\".  See if we have\n \t * 0, 1 or more objects that actually match(es).\n \t */\n-\tfor (i = first; i < num; i++) {\n-\t\tcurrent = nth_packed_object_sha1(p, first);\n+\tfor (i = first; i < num && !ds->ambiguous; i++) {\n+\t\tcurrent = nth_packed_object_sha1(p, i);\n \t\tif (!match_sha(len, bin_pfx, current))\n \t\t\tbreak;\n-\n-\t\t/* current matches */\n-\t\tif (!seen_so_far) {\n-\t\t\t*found_sha1 = current;\n-\t\t\tseen_so_far++;\n-\t\t} else if (seen_so_far) {\n-\t\t\t/* is it the same as the one previously found elsewhere? */\n-\t\t\tif (hashcmp(*found_sha1, current))\n-\t\t\t\treturn 2; /* definitely not unique */\n-\t\t}\n+\t\tupdate_candidates(ds, current);\n \t}\n-\treturn seen_so_far;\n }\n \n-static int find_short_packed_object(int len, const unsigned char *bin_pfx, unsigned char *sha1)\n+static void find_short_packed_object(int len, const unsigned char *bin_pfx,\n+\t\t\t\t     struct disambiguate_state *ds)\n {\n \tstruct packed_git *p;\n-\tconst unsigned char *found_sha1 = NULL;\n-\tint found = 0;\n \n \tprepare_packed_git();\n-\tfor (p = packed_git; p && found < 2; p = p->next)\n-\t\tfound = unique_in_pack(len, bin_pfx, p, &found_sha1, found);\n-\n-\tif (found == 1)\n-\t\thashcpy(sha1, found_sha1);\n-\treturn found;\n+\tfor (p = packed_git; p && !ds->ambiguous; p = p->next)\n+\t\tunique_in_pack(len, bin_pfx, p, ds);\n }\n \n #define SHORT_NAME_NOT_FOUND (-1)\n #define SHORT_NAME_AMBIGUOUS (-2)\n \n-static int find_unique_short_object(int len, char *hex_pfx,\n-\t\t\t\t    unsigned char *bin_pfx, unsigned char *sha1)\n+static int finish_object_disambiguation(struct disambiguate_state *ds,\n+\t\t\t\t\tunsigned char *sha1)\n {\n-\tint has_unpacked, has_packed;\n-\tunsigned char unpacked_sha1[20], packed_sha1[20];\n+\tif (ds->ambiguous)\n+\t\treturn SHORT_NAME_AMBIGUOUS;\n \n-\tprepare_alt_odb();\n-\thas_unpacked = find_short_object_filename(len, hex_pfx, unpacked_sha1);\n-\thas_packed = find_short_packed_object(len, bin_pfx, packed_sha1);\n-\tif (!has_unpacked && !has_packed)\n+\tif (!ds->candidate_exists)\n \t\treturn SHORT_NAME_NOT_FOUND;\n-\tif (1 < has_unpacked || 1 < has_packed)\n-\t\treturn SHORT_NAME_AMBIGUOUS;\n-\tif (has_unpacked != has_packed) {\n-\t\thashcpy(sha1, (has_packed ? packed_sha1 : unpacked_sha1));\n-\t\treturn 0;\n-\t}\n-\t/* Both have unique ones -- do they match? */\n-\tif (hashcmp(packed_sha1, unpacked_sha1))\n-\t\treturn SHORT_NAME_AMBIGUOUS;\n-\thashcpy(sha1, packed_sha1);\n+\n+\tif (!ds->candidate_checked)\n+\t\t/*\n+\t\t * If this is the only candidate, there is no point\n+\t\t * calling the disambiguation hint callback.\n+\t\t *\n+\t\t * On the other hand, if the current candidate\n+\t\t * replaced an earlier candidate that did _not_ pass\n+\t\t * the disambiguation hint callback, then we do have\n+\t\t * more than one objects that match the short name\n+\t\t * given, so we should make sure this one matches;\n+\t\t * otherwise, if we discovered this one and the one\n+\t\t * that we previously discarded in the reverse order,\n+\t\t * we would end up showing different results in the\n+\t\t * same repository!\n+\t\t */\n+\t\tds->candidate_ok = (!ds->disambiguate_fn_used ||\n+\t\t\t\t    ds->fn(ds->candidate, ds->cb_data));\n+\n+\tif (!ds->candidate_ok)\n+\t\treturn SHORT_NAME_NOT_FOUND;\n+\n+\thashcpy(sha1, ds->candidate);\n \treturn 0;\n }\n \n@@ -179,6 +224,7 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \tint i, status;\n \tchar hex_pfx[40];\n \tunsigned char bin_pfx[20];\n+\tstruct disambiguate_state ds;\n \n \tif (len < MINIMUM_ABBREV || len > 40)\n \t\treturn -1;\n@@ -203,7 +249,13 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \t\tbin_pfx[i >> 1] |= val;\n \t}\n \n-\tstatus = find_unique_short_object(i, hex_pfx, bin_pfx, sha1);\n+\tprepare_alt_odb();\n+\n+\tmemset(&ds, 0, sizeof(ds));\n+\tfind_short_object_filename(len, hex_pfx, &ds);\n+\tfind_short_packed_object(len, bin_pfx, &ds);\n+\tstatus = finish_object_disambiguation(&ds, sha1);\n+\n \tif (!quietly && (status == SHORT_NAME_AMBIGUOUS))\n \t\treturn error(\"short SHA1 %.*s is ambiguous.\", len, hex_pfx);\n \treturn status;\n-- \n1.7.11.2.gd284367\n"},{"id":"194026","messageId":"1340260532-11378-8-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 7/9] sha1_name.c: allow get_short_sha1() to take other flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:30Z","receivedAt":"2012-06-21T06:35:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Instead of a separate \"int quietly\" argument, make it take \"unsigned flags\"\nso that we can pass other options to it.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 852d5c2..7cca7d9 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -218,13 +218,16 @@ static int finish_object_disambiguation(struct disambiguate_state *ds,\n \treturn 0;\n }\n \n+#define GET_SHORT_QUIETLY 01\n+\n static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n-\t\t\t  int quietly)\n+\t\t\t  unsigned flags)\n {\n \tint i, status;\n \tchar hex_pfx[40];\n \tunsigned char bin_pfx[20];\n \tstruct disambiguate_state ds;\n+\tint quietly = !!(flags & GET_SHORT_QUIETLY);\n \n \tif (len < MINIMUM_ABBREV || len > 40)\n \t\treturn -1;\n@@ -272,7 +275,7 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)\n \t\treturn hex;\n \twhile (len < 40) {\n \t\tunsigned char sha1_ret[20];\n-\t\tstatus = get_short_sha1(hex, len, sha1_ret, 1);\n+\t\tstatus = get_short_sha1(hex, len, sha1_ret, GET_SHORT_QUIETLY);\n \t\tif (exists\n \t\t    ? !status\n \t\t    : status == SHORT_NAME_NOT_FOUND) {\n@@ -603,7 +606,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1)\n \t\t\tif (ch == 'g' && cp[-1] == '-') {\n \t\t\t\tcp++;\n \t\t\t\tlen -= cp - name;\n-\t\t\t\treturn get_short_sha1(cp, len, sha1, 1);\n+\t\t\t\treturn get_short_sha1(cp, len, sha1, GET_SHORT_QUIETLY);\n \t\t\t}\n \t\t}\n \t}\n-- \n1.7.11.2.gd284367\n"},{"id":"194022","messageId":"1340260532-11378-9-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 8/9] sha1_name.c: teach get_short_sha1() a commit-only option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:31Z","receivedAt":"2012-06-21T06:35:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 7cca7d9..58dbbe2 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -219,6 +219,13 @@ static int finish_object_disambiguation(struct disambiguate_state *ds,\n }\n \n #define GET_SHORT_QUIETLY 01\n+#define GET_SHORT_COMMIT_ONLY 02\n+\n+static int disambiguate_commit_only(const unsigned char *sha1, void *cb_data_unused)\n+{\n+\tint kind = sha1_object_info(sha1, NULL);\n+\treturn kind == OBJ_COMMIT;\n+}\n \n static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \t\t\t  unsigned flags)\n@@ -255,6 +262,9 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,\n \tprepare_alt_odb();\n \n \tmemset(&ds, 0, sizeof(ds));\n+\tif (flags & GET_SHORT_COMMIT_ONLY)\n+\t\tds.fn = disambiguate_commit_only;\n+\n \tfind_short_object_filename(len, hex_pfx, &ds);\n \tfind_short_packed_object(len, bin_pfx, &ds);\n \tstatus = finish_object_disambiguation(&ds, sha1);\n-- \n1.7.11.2.gd284367\n"},{"id":"194028","messageId":"1340260532-11378-10-git-send-email-gitster@pobox.com","threadId":"30857","inReplyTo":"1340260532-11378-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 9/9] sha1_name.c: get_describe_name() by definition groks only commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T06:35:32Z","receivedAt":"2012-06-21T06:35:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 58dbbe2..15e97eb 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -606,6 +606,7 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n static int get_describe_name(const char *name, int len, unsigned char *sha1)\n {\n \tconst char *cp;\n+\tunsigned flags = GET_SHORT_QUIETLY | GET_SHORT_COMMIT_ONLY;\n \n \tfor (cp = name + len - 1; name + 2 <= cp; cp--) {\n \t\tchar ch = *cp;\n@@ -616,7 +617,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1)\n \t\t\tif (ch == 'g' && cp[-1] == '-') {\n \t\t\t\tcp++;\n \t\t\t\tlen -= cp - name;\n-\t\t\t\treturn get_short_sha1(cp, len, sha1, GET_SHORT_QUIETLY);\n+\t\t\t\treturn get_short_sha1(cp, len, sha1, flags);\n \t\t\t}\n \t\t}\n \t}\n-- \n1.7.11.2.gd284367\n"},{"id":"194033","messageId":"CABPQNSZ246Y80WJN+u2OR8_3KWE_1=8F6WbTgVX-8wqyecGPkg@mail.gmail.com","threadId":"30857","inReplyTo":"1340260532-11378-10-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v2 9/9] sha1_name.c: get_describe_name() by definition groks only commits","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-06-21T09:22:07Z","receivedAt":"2012-06-21T09:22:07Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Thu, Jun 21, 2012 at 8:35 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  sha1_name.c | 3 ++-\n>  1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/sha1_name.c b/sha1_name.c\n> index 58dbbe2..15e97eb 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -606,6 +606,7 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n>  static int get_describe_name(const char *name, int len, unsigned char *sha1)\n>  {\n>        const char *cp;\n> +       unsigned flags = GET_SHORT_QUIETLY | GET_SHORT_COMMIT_ONLY;\n>\n>        for (cp = name + len - 1; name + 2 <= cp; cp--) {\n>                char ch = *cp;\n> @@ -616,7 +617,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1)\n>                        if (ch == 'g' && cp[-1] == '-') {\n>                                cp++;\n>                                len -= cp - name;\n> -                               return get_short_sha1(cp, len, sha1, GET_SHORT_QUIETLY);\n> +                               return get_short_sha1(cp, len, sha1, flags);\n\nIs there a reason why you chose to put the definition in the\nroot-scope of the function? There's an excellent opportunity just a\nfew lines above the only place it's used, and I suspect it would\nincrease readability to put it there...\n"},{"id":"194049","messageId":"7v1ul8sajk.fsf@alter.siamese.dyndns.org","threadId":"30857","inReplyTo":"CABPQNSZ246Y80WJN+u2OR8_3KWE_1=8F6WbTgVX-8wqyecGPkg@mail.gmail.com","subject":"Re: [PATCH v2 9/9] sha1_name.c: get_describe_name() by definition groks only commits","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-21T19:57:19Z","receivedAt":"2012-06-21T19:57:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Erik Faye-Lund <kusmabite@gmail.com> writes:\n\n> On Thu, Jun 21, 2012 at 8:35 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n>> ---\n>>  sha1_name.c | 3 ++-\n>>  1 file changed, 2 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/sha1_name.c b/sha1_name.c\n>> index 58dbbe2..15e97eb 100644\n>> --- a/sha1_name.c\n>> +++ b/sha1_name.c\n>> @@ -606,6 +606,7 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n>>  static int get_describe_name(const char *name, int len, unsigned char *sha1)\n>>  {\n>>        const char *cp;\n>> +       unsigned flags = GET_SHORT_QUIETLY | GET_SHORT_COMMIT_ONLY;\n>>\n>>        for (cp = name + len - 1; name + 2 <= cp; cp--) {\n>>                char ch = *cp;\n>> @@ -616,7 +617,7 @@ static int get_describe_name(const char *name, int len, unsigned char *sha1)\n>>                        if (ch == 'g' && cp[-1] == '-') {\n>>                                cp++;\n>>                                len -= cp - name;\n>> -                               return get_short_sha1(cp, len, sha1, GET_SHORT_QUIETLY);\n>> +                               return get_short_sha1(cp, len, sha1, flags);\n>\n> Is there a reason why you chose to put the definition in the\n> root-scope of the function?\n\nMostly because it never changes inside the loop.  \"unsigned const\nflags\" upfront might have even been a better option, though.\n\nA less important reason is it would make the resulting file wider,\nnot narrower, than the original.\n"}]}