On Fri, Apr 05, 2019 at 09:21:25AM -0400, Jeff King wrote:
> > ... which also means this open_pack_index() call isn't done anymore if
> > there's no .midx file at all. You don't mention this change in the
> > commit message; is it intended?
>
> Doh, thank you for catching that. I made that switch at the last minute
> because I didn't want to pay the malloc/free cost when we had no list to
> compare to. I'm surprised it works at all. :-/
>
> I guess it doesn't, from the other message in the thread.
Hmm, no, that's a different problem. I think we'd generally already have
the pack open unless there _is_ a midx, so this conditional probably
never kicks in unless we have a midx anyway. Meaning my malloc-avoidance
was over-thought. So we could just get rid of it. Or do it like this:
diff --git a/packfile.c b/packfile.c
index e7ca135ed5..ef0f959311 100644
--- a/packfile.c
+++ b/packfile.c
@@ -496,17 +496,22 @@ static int open_packed_git_1(struct packed_git *p)
ssize_t read_result;
const unsigned hashsz = the_hash_algo->rawsz;
- if (!p->index_data && the_repository->objects->multi_pack_index) {
- struct multi_pack_index *m;
- char *idx_name = pack_name_to_idx(pack_basename(p));
+ if (!p->index_data) {
+ struct multi_pack_index *m =
+ the_repository->objects->multi_pack_index;
- for (m = the_repository->objects->multi_pack_index;
- m; m = m->next) {
- if (midx_contains_pack(m, idx_name))
- break;
+ if (m) {
+ char *idx_name = pack_name_to_idx(pack_basename(p));
+
+ for (; m; m = m->next) {
+ if (midx_contains_pack(m, idx_name))
+ break;
+ }
+ free(idx_name);
}
- free(idx_name);
+ if (!m)
+ warning("opening anyway");
if (!m && open_pack_index(p))
return error("packfile %s index unavailable", p->pack_name);
}
I'll have to re-roll to address the other problem, though, so I'll give
it some thought.
-Peff
