Re: [PATCH v3 5/5] sha1_name: Minimize OID comparisons during disambiguation

On 10/3/2017 11:55 AM, Stefan Beller wrote:

quoted

@@ -505,6 +506,65 @@ static int extend_abbrev_len(const struct object_id *oid, void *cb_data)
         return 0;
  }

+static void find_abbrev_len_for_pack(struct packed_git *p,
+                                    struct min_abbrev_data *mad)
+{
+       int match = 0;
+       uint32_t num, last, first = 0;
+       struct object_id oid;
+
+       open_pack_index(p);

coverity complained here with
     Calling "open_pack_index" without checking return value
     (as is done elsewhere 13 out of 15 times).

Good catch! This same line is repeated in unique_in_pack() in this same 
file, so if this is worth fixing then we should probably fix it there, too.

I think the easiest way out is just a

     if (open_pack_index(p))
         die(_("Cannot open existing pack idx file for '%s'"), p);

or is there another good approach?

You probably intended to have p->pack_name in the die();

Using `cat *.c | grep -A 2 "if (open_pack_index("` and `cat */*.c | grep 
-A 2 "if (open_pack_index("` I see a few places that return error codes 
or quietly fail. The cases that use die() are inside builtin/ so I don't 
think die() is the right choice here.

Since find_abbrev_len_for_pack() is intended to extend the abbreviation 
length when necessary, I think a silent return is best here:

     if (open_pack_index(p))
         return;

Thanks,
-Stolee