@matheusbsbs / matheus-bagliano / commits / 8a10fea49b

get_sha1: propagate flags to child functions

The get_sha1() function is actually implementation by many sub-functions, but we do not always pass our flags around to all of those functions. As a result, we may forget that our caller asked us to resolve with GET_SHA1_QUIETLY and output messages. The two triggerable cases are: 1. Resolving treeish:path will resolve the "treeish" portion using GET_SHA1_TREEISH, dropping all other flags. 2. The peel_onion() function did not take flags at all but recurses to get_sha1_1(), which does. The solution for both is to bitwise-OR their new flags with the existing ones (after dropping any mutually exclusive disambiguation flags). This bug can trigger with "git rev-parse --quiet", which asks for quiet resolution. But it can also happen in a more vanilla code path when we do a follow-up ONLY_TO_DIE invocation of get_sha1(), and that's what the tests check. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 26, 2016 at 07:59 UTC 8a10fea49b1f95daeea445072a60139590b216cf

2 files changed +25 -7

sha1_name.c

+12 -6

@@ -681,12 +681,12 @@ struct object *peel_to_type(const char *name, int namelen,

681 }

682 }

683

684 -static int peel_onion(const char *name, int len, unsigned char *sha1)

684 +static int peel_onion(const char *name, int len, unsigned char *sha1,

685 + unsigned lookup_flags)

686 {

687 unsigned char outer[20];

688 const char *sp;

689 unsigned int expected_type = 0;

689 - unsigned lookup_flags = 0;

690 struct object *o;

691

692 /*

@@ -726,10 +726,11 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)

726 else

727 return -1;

728

729 + lookup_flags &= ~GET_SHA1_DISAMBIGUATORS;

730 if (expected_type == OBJ_COMMIT)

730 - lookup_flags = GET_SHA1_COMMITTISH;

731 + lookup_flags |= GET_SHA1_COMMITTISH;

732 else if (expected_type == OBJ_TREE)

732 - lookup_flags = GET_SHA1_TREEISH;

733 + lookup_flags |= GET_SHA1_TREEISH;

734

735 if (get_sha1_1(name, sp - name - 2, outer, lookup_flags))

736 return -1;

@@ -830,7 +831,7 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l

831 return get_nth_ancestor(name, len1, sha1, num);

832 }

833

833 - ret = peel_onion(name, len, sha1);

834 + ret = peel_onion(name, len, sha1, lookup_flags);

835 if (!ret)

836 return 0;

837

@@ -1465,7 +1466,12 @@ static int get_sha1_with_context_1(const char *name,

1466 if (*cp == ':') {

1467 unsigned char tree_sha1[20];

1468 int len = cp - name;

1468 - if (!get_sha1_1(name, len, tree_sha1, GET_SHA1_TREEISH)) {

1469 + unsigned sub_flags = flags;

1470 +

1471 + sub_flags &= ~GET_SHA1_DISAMBIGUATORS;

1472 + sub_flags |= GET_SHA1_TREEISH;

1473 +

1474 + if (!get_sha1_1(name, len, tree_sha1, sub_flags)) {

1475 const char *filename = cp+1;

1476 char *new_filename = NULL;

1477

t/t1512-rev-parse-disambiguation.sh

+13 -1

@@ -291,10 +291,22 @@ test_expect_success 'ambiguous short sha1 ref' '

291 grep "refname.*${REF}.*ambiguous" err

292 '

293

294 -test_expect_success C_LOCALE_OUTPUT 'ambiguity errors are not repeated' '

294 +test_expect_success C_LOCALE_OUTPUT 'ambiguity errors are not repeated (raw)' '

295 test_must_fail git rev-parse 00000 2>stderr &&

296 grep "is ambiguous" stderr >errors &&

297 test_line_count = 1 errors

298 '

299

300 +test_expect_success C_LOCALE_OUTPUT 'ambiguity errors are not repeated (treeish)' '

301 + test_must_fail git rev-parse 00000:foo 2>stderr &&

302 + grep "is ambiguous" stderr >errors &&

303 + test_line_count = 1 errors

304 +'

305 +

306 +test_expect_success C_LOCALE_OUTPUT 'ambiguity errors are not repeated (peel)' '

307 + test_must_fail git rev-parse 00000^{commit} 2>stderr &&

308 + grep "is ambiguous" stderr >errors &&

309 + test_line_count = 1 errors

310 +'

311 +

312 test_done