Re: [PATCH GSoC v2 4/6] fetch-object-info: parse type from server response

On Sat Aug 1, 2026 at 7:04 AM CEST, Junio C Hamano wrote:

Pablo Sabater [off-list ref] writes:

quoted

@@ -104,8 +105,13 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
 			for (size_t j = 0; j < args->oids->nr; j++)
 				object_info_data[j].sizep =
 					xcalloc(1, sizeof(*object_info_data[j].sizep));
+		} else if (!strcmp(reader->line, "type")) {
+			type_index = (int)i;
+			for (size_t j = 0; j < args->oids->nr; j++)
+				object_info_data[j].typep =
+					xcalloc(1, sizeof(*object_info_data[j].typep));

Do object_info_data[j].typep and object_info_data[k].typep need to
be independently freeable?  Separate allocations by calling calloc
args->oids->nr times would allow that, but if there is no such need,
nr contiguous allocation of them,

		enum object_type *types;

		*types = xcalloc(args->oids->nr, sizeof(*types));
		for (size_t j = 0; j < args->oids->nr; j++)
			object_info_data[j].typep = &types[j];

would be simpler to manage and easier to get rid of once you are
done.

Hmmmm, they don't need to be independently freeable but they are freed
by free_object_info_contents() called at the end of
parse_cmd_remote_object_info() at 'builtin/cat-file.c' in a loop:

	for (size_t i = 0; i < object_info_oids.nr; i++)
		free_object_info_contents(&remote_object_info[i]);

free_object_info_contents() is:

	void free_object_info_contents(struct object_info *object_info)
	{
		if (!object_info)
			return;
		free(object_info->typep);
		free(object_info->sizep);
		free(object_info->disk_sizep);
		free(object_info->delta_base_oid);
	}

This function was implemented by the series that introduced
remote-object-info (the one that this series is based on) so
parse_cmd_remote_object_info() is the only caller.

Thinking about it, your suggestion can be done easily. To free types it
is enough to do free(remote_object_info[0].typep); (same for sizep).

I'll make it work as a prep patch for size and modify this one to do the
same.
free_object_info_contents() gets dropped in the prep patch because it
would have no callers after it.

quoted

 		} else {
-			BUG("only size is supported");
+			BUG("unexpected object-info option: %s", reader->line);
 		}
 	}

@@ -151,6 +157,10 @@ int fetch_object_info(const enum protocol_version version, struct object_info_ar
 			    object_info_values.items[0].string,
 			    object_info_values.items[size_index + 1].string);

+		if (type_index >= 0)
+			*object_info_data[i].typep =
+				type_from_string(object_info_values.items[type_index + 1].string);
+
 		string_list_clear(&object_info_values, 0);
 	}
 	check_stateless_delimiter(stateless_rpc, reader, "stateless delimiter expected");

Thanks for the review,
Pablo