Taylor Blau <me@xxxxxxxxxxxx> writes:
> The function midx.c:verify_midx_file() allocate a MIDX struct by calling
> load_multi_pack_index(). But when cleaning up, it calls free() without
> freeing any resources associated with the MIDX.
>
> Call the more appropriate close_midx() which does free those resources,
> which causes t5319.3 to pass when Git is compiled with SANITIZE=leak.
Nice.
By the way, the function starts like so:
int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags)
{
struct pair_pos_vs_id *pairs = NULL;
uint32_t i;
struct progress *progress = NULL;
struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
verify_midx_error = 0;
if (!m) {
int result = 0;
struct stat sb;
char *filename = get_midx_filename(object_dir);
if (!stat(filename, &sb)) {
error(_("multi-pack-index file exists, but failed to parse"));
result = 1;
}
free(filename);
return result;
}
but after seeing die() sprinkled in load_multi_pack_index() with
checks during parsing, I am not sure if this is a good error
reporting mechanism we are seeing here.
It is wonderful to plug leaks here and there, but to be honest, even
with only a very little parts I saw in this code, I think there are
other things that need clean-up here.
Also, the way the file-scope global verify_midx_error is used is
beyond words _ugly_, if the only reason for its use is to make
midx_report() look simpler, which is what I think is happening.
Not very impressed, but all of the above is not a new issue
introduced by this patch.
Thanks.
