On 2/28/2018 3:50 PM, Junio C Hamano wrote:
Derrick Stolee <dsto...@microsoft.com> writes:diff --git a/sha1_name.c b/sha1_name.c index 611c7d24dd..a041d8d24f 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -547,15 +547,15 @@ static void find_abbrev_len_for_pack(struct packed_git *p, */ mad->init_len = 0; if (!match) { - nth_packed_object_oid(&oid, p, first); - extend_abbrev_len(&oid, mad); + if (nth_packed_object_oid(&oid, p, first)) + extend_abbrev_len(&oid, mad); } else if (first < num - 1) { - nth_packed_object_oid(&oid, p, first + 1); - extend_abbrev_len(&oid, mad); + if (nth_packed_object_oid(&oid, p, first + 1)) + extend_abbrev_len(&oid, mad); } if (first > 0) { - nth_packed_object_oid(&oid, p, first - 1); - extend_abbrev_len(&oid, mad); + if (nth_packed_object_oid(&oid, p, first - 1)) + extend_abbrev_len(&oid, mad); } mad->init_len = mad->cur_len; }I do not think they are wrong, but aren't the latter two somewhat redundant? "num" is p->num_objects, and we call (first+1)th element only after we see (first < num - 1), i.e. first+1 < num, and the access to (first-1)th is done only when first > 0. The first one, i.e. when first points at where we _would_ find it if it existed, can access "first" that could be p->num_objects, so the change there makes sense, though.
Yes. But I'd rather keep the blocks consistent and use the return value of nth_packed_object_oid() when possible.
Thanks, -Stolee
