Re: [PATCH v3 0/3]

[Jeff King <peff@peff.net>]

On Wed, Jun 30, 2021 at 04:06:13PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Refactor the bundle API to use the string_list API instead of its own
> version of a similar API. See [1] for v2.
> 
> Addresses comments by Jeff King about us being too overzelous in
> trying not to leak memory (the 'die_no_repo' is gone), and other
> flow/style comments of his.
> 
> I also added a bundle_header_init() function for use in transport.c,
> and noticed a redundant call to string_list_clear() there.

Thanks, all three look good to me.

As an aside, having to have a separate bundle_header_init() and
BUNDLE_HEADER_INIT is annoying (because they both must be kept up to
date with each other), but quite common in our code base. I wonder if
writing:

  void bundle_header_init(struct bundle_header *header)
  {
	struct bundle_header blank = BUNDLE_HEADER_INIT;
	memcpy(header, &blank, sizeof(*header));
  }

would let a smart enough compiler just init "header" in place without
the extra copy (the performance of a single bundle_header almost
certainly doesn't matter, but it might for other types).

Just musing. ;)

-Peff