Re: [PATCH 2/2] upload-pack: use stdio in send_ref callbacks

[Jeff King <peff@peff.net>]

On Thu, Aug 26, 2021 at 12:06:48PM +0200, Jacob Vosmaer wrote:

> @@ -126,6 +127,7 @@ static void upload_pack_data_init(struct upload_pack_data *data)
>  	struct string_list uri_protocols = STRING_LIST_INIT_DUP;
>  	struct object_array extra_edge_obj = OBJECT_ARRAY_INIT;
>  	struct string_list allowed_filters = STRING_LIST_INIT_DUP;
> +	struct strbuf send_ref_buf = STRBUF_INIT;
>  
>  	memset(data, 0, sizeof(*data));
>  	data->symref = symref;
> @@ -141,6 +143,7 @@ static void upload_pack_data_init(struct upload_pack_data *data)
>  	data->allow_filter_fallback = 1;
>  	data->tree_filter_max_depth = ULONG_MAX;
>  	packet_writer_init(&data->writer, 1);
> +	data->send_ref_buf = send_ref_buf;

This does a struct copy of the strbuf, which is usually a bad thing
(both copies think they own the pointer). It works here because the
original immediately goes out of scope. The usual thing would be to do
this instead:

  strbuf_init(&data->send_ref_buf, 0);

but I notice this whole function is somewhat odd that way (lots of
static initializers followed by struct assignment, rather than using
initialization functions).

I'm not sure if it's worth changing or not. Again, I don't think it's
doing the wrong thing, but just sort of unusual for our code base. A few
of the data structures in use here don't have _init() functions
(object_array and oid_array), but that probably means we ought to add
them.

-Peff