> Junio C Hamano <gitster@pobox.com> writes:
> 
> > When merged to the tip of 'seen' (with a fixup to use st_mtime where
> > we need only whole second precision, to avoid using st_mtim on
> > platforms that do not have it), this seems to break linux-leaks and
> > linux-reftable-leaks CI jobs (t0410, t5616, and t5710).
> >
> > This topic standalone, without interaction with other things in
> > 'seen', breaks these three tests.
> >
> >   https://github.com/git/git/actions/runs/24267948258/job/70866907548
> >
> > This is one commit directly on top of your topic that reduces CI
> > jobs down to just two "leaks" job, and removes many test scripts
> > to leave only these three breaking ones.
> >
> >
> > I managed to also locally reproduce this failure.  Here is how
> >
> >     $ cd t && t5710-*.sh -i -v
> >
> > dies:
> >
> > Direct leak of 285 byte(s) in 1 object(s) allocated from:
> >     #0 0x55ce48ca1d4d in malloc (git+0x8cd4d) (BuildId: 1aa6efa30b2fc4772028a3dd31aba3ced49bf128)
> >     #1 0x55ce48fed3f2 in do_xmalloc wrapper.c:55:8
> >     #2 0x55ce48fed3b6 in xmalloc wrapper.c:76:9
> >     #3 0x55ce48eed314 in alloc_packed_git packfile.c:306:25
> >     #4 0x55ce48eed209 in parse_pack_index packfile.c:326:25
> >     #5 0x55ce48f65f03 in copy_promisor_content repack-promisor.c:67:14
> >     ...
> 
> 
> FWIW, the tip of 'seen' as of this writing has queued this topic
> (the latest round v5) plus the attached "SQUASH???" commit at its
> tip.
> 
> The first hunk (die when dest_pack is NULL) is absolutely positively
> a wrong thing to do.  As I said, I do not know if we want to call
> parse_pack_index() here or if we have a more appropriate helper
> function to use, but assuming that parse_pack_index() is the right
> thing to call, we should be prepared to see NULL returned.  BUG("")
> is reserved for detected programming errors, and it is absolutely a
> wrong thing to call.
>
> As the content copied by this function is supposed to be for
> debugging only, I think dying when we cannot copy is not what we
> want.  Rather, it probably makes more sense to fall back to the
> traditional behaviour (e.g., not copying and instead leaving an
> empty file, if that is what we did before this patch series).

> The second hunk just line-wraps an overly long line.  There are
> other overly long lines in this deeply indented block (which is a
> sign that it might be worth to see if the block is better made into
> a small static helper function) that should be line-wrapped in a
> similar way, but I didn't bother.

> 
>  repack-promisor.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/repack-promisor.c b/repack-promisor.c
> index 26055212a3..c7025e97f2 100644
> --- a/repack-promisor.c
> +++ b/repack-promisor.c
> @@ -71,6 +71,8 @@ static void copy_promisor_content(struct repository *repo,
>  	dest_idx_name = mkpathdup("%s-%s.idx", packtmp, dest_hex);
>  	get_oid_hex_algop(dest_hex, &dest_oid, repo->hash_algo);
>  	dest_pack = parse_pack_index(repo, dest_oid.hash, dest_idx_name);
> +	if (!dest_pack)
> +		BUG("parse_pack_index() failed.");
>  
>  	/* Open the .promisor dest file, and fill dest_content with its content */
>  	dest_promisor_name = mkpathdup("%s-%s.promisor", packtmp, dest_hex);
> @@ -115,7 +117,8 @@ static void copy_promisor_content(struct repository *repo,
>  
>  			/* If <time> doesn't exist, retrieve it and add it to line */
>  			if (line_sections.nr < 3)
> -				strbuf_addf(&line, " %" PRItime, (timestamp_t)source_stat.st_mtime);
> +				strbuf_addf(&line, " %" PRItime,
> +					    (timestamp_t)source_stat.st_mtime);
>  
>  			/*
>  			 * Add the finalized line to dest_to_write and dest_content if it
> @@ -148,6 +151,7 @@ static void copy_promisor_content(struct repository *repo,
>  		die(_("Could not write '%s' promisor file"), dest_promisor_name);
>  
>  	close_pack_index(dest_pack);
> +	free(dest_pack);
>  	free(dest_idx_name);
>  	free(dest_promisor_name);
>  	strset_clear(&dest_content);
> --