Re: [PATCH v4 6/6] fetch: delay user information post committing of transaction
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jan 22, 2026, 20:10 UTC
- Message-ID
- <xmqqldhpmmrw.fsf@gitster.g>
- In-Reply-To
- <20260122-633-regression-lost-diagnostic-message-when-pushing-non-commit-objects-to-refs-heads-v4-6-2ddba0832440@gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 17 quoted lines
> +struct ref_update_display_info {
> + bool failed;
> + char success_code;
> + char fail_code;
> + const char *summary;
> + const char *fail_detail;
> + const char *success_detail;
> + const char *ref;
> + const char *remote;
> + struct object_id old_oid;
> + struct object_id new_oid;
> +};
> +
> +struct ref_update_display_info_array {
> + struct ref_update_display_info *info;
> + size_t alloc, nr;
> +};OK. The ref_update_display_info structure is full of pointers. They are of "const char *" type, hinting that they are borrowed pieces of memory, and there is nothing to clean inside, other than the .info member itself?
Show 12 quoted lines
> +static struct ref_update_display_info *ref_update_display_info_append(
> + struct ref_update_display_info_array *array,
> + char success_code,
> + char fail_code,
> + const char *summary,
> + const char *success_detail,
> + const char *fail_detail,
> + const char *ref,
> + const char *remote,
> + const struct object_id *old_oid,
> + const struct object_id *new_oid)
> +{This helper that consumes the structure is used throughout the patch, and relative to the previous round it got easier to read.
Show 8 quoted lines
> +static void ref_update_display_info_free(struct ref_update_display_info *info)
> +{
> + free((char *)info->summary);
> + free((char *)info->success_detail);
> + free((char *)info->fail_detail);
> + free((char *)info->remote);
> + free((char *)info->ref);
> +}This answers "no" to my previous question. These are not borrowed, but are owned by this structure.
Show 16 quoted lines
> @@ -1965,7 +2090,17 @@ static int do_fetch(struct transport *transport,
> */
> if (retcode && !atomic_fetch && transaction)
> commit_ref_transaction(&transaction, false,
> - transport->remote->name, &err);
> + transport->remote->name,
> + &rejected_refs, &err);
> +
> + for (size_t i = 0; i < display_array.nr; i++) {
> + struct ref_update_display_info *info = &display_array.info[i];
> +
> + if (!info->failed && strmap_contains(&rejected_refs, info->ref))
> + ref_update_display_info_set_failed(info);
> + ref_update_display_info_display(info, &display_state, summary_width);
> + ref_update_display_info_free(info);
> + }And after a fetch finishes and we consume the display_info, we call _free() to release the resource held there, plus ...
Show 8 quoted lines
> if (retcode) {
> if (err.len) {
> @@ -1980,6 +2115,9 @@ static int do_fetch(struct transport *transport,
>
> if (transaction)
> ref_transaction_free(transaction);
> +
> + free(display_array.info);... of course the array itself, which makes sense.