midx: check both pack and index names for containment

A midx file (and the struct we parse from it) contains a list of all of the covered packfiles, mentioned by their ".idx" names (e.g., "pack-1234.idx", etc). And thus calls to midx_contains_pack() expect callers to provide the idx name. This works for most of the calls, but the one in open_packed_git_1() tries to feed a packed_git->pack_name, which is the ".pack" name, meaning we'll never find a match (even if the pack is covered by the midx). We can fix this by converting the ".pack" to ".idx" in the caller. However, that requires allocating a new string. Instead, let's make midx_contains_pack() a bit friendlier, and allow it take _either_ the .pack or .idx variant. All cleverness in the matching code is credited to René. Bugs are mine. There's no test here, because while this does fix _a_ bug, it's masked by another bug in that same caller. That will be covered (with a test) in the next patch. Helped-by: René Scharfe <l.s.r@web.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Apr 5, 2019 at 14:06 UTC 013fd7ada3c81cec8f0c48427c77394028707c2e
2 files changed +35 -3
midx.c
+34 -2
@@ -307,7 +307,39 @@ int fill_midx_entry(const struct object_id *oid, struct pack_entry *e, struct mu
307 return nth_midxed_pack_entry(m, e, pos);
308 }
309
310 -int midx_contains_pack(struct multi_pack_index *m, const char *idx_name)
310 +/* Match "foo.idx" against either "foo.pack" _or_ "foo.idx". */
311 +static int cmp_idx_or_pack_name(const char *idx_or_pack_name,
312 + const char *idx_name)
313 +{
314 + /* Skip past any initial matching prefix. */
315 + while (*idx_name && *idx_name == *idx_or_pack_name) {
316 + idx_name++;
317 + idx_or_pack_name++;
318 + }
319 +
320 + /*
321 + * If we didn't match completely, we may have matched "pack-1234." and
322 + * be left with "idx" and "pack" respectively, which is also OK. We do
323 + * not have to check for "idx" and "idx", because that would have been
324 + * a complete match (and in that case these strcmps will be false, but
325 + * we'll correctly return 0 from the final strcmp() below.
326 + *
327 + * Technically this matches "fooidx" and "foopack", but we'd never have
328 + * such names in the first place.
329 + */
330 + if (!strcmp(idx_name, "idx") && !strcmp(idx_or_pack_name, "pack"))
331 + return 0;
332 +
333 + /*
334 + * This not only checks for a complete match, but also orders based on
335 + * the first non-identical character, which means our ordering will
336 + * match a raw strcmp(). That makes it OK to use this to binary search
337 + * a naively-sorted list.
338 + */
339 + return strcmp(idx_or_pack_name, idx_name);
340 +}
341 +
342 +int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name)
343 {
344 uint32_t first = 0, last = m->num_packs;
345
@@ -317,7 +349,7 @@ int midx_contains_pack(struct multi_pack_index *m, const char *idx_name)
349 int cmp;
350
351 current = m->pack_names[mid];
320 - cmp = strcmp(idx_name, current);
352 + cmp = cmp_idx_or_pack_name(idx_or_pack_name, current);
353 if (!cmp)
354 return 1;
355 if (cmp > 0) {
midx.h
+1 -1
@@ -43,7 +43,7 @@ struct object_id *nth_midxed_object_oid(struct object_id *oid,
43 struct multi_pack_index *m,
44 uint32_t n);
45 int fill_midx_entry(const struct object_id *oid, struct pack_entry *e, struct multi_pack_index *m);
46 -int midx_contains_pack(struct multi_pack_index *m, const char *idx_name);
46 +int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name);
47 int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);
48
49 int write_midx_file(const char *object_dir);