Subject: Re: [PATCH v2 6/6] update-ref: add support for 'symref-update' command

From: Junio C Hamano <[email-masked]>
Date: Sat, 25 May 2024 16:00:45 -0700

Karthik Nayak <[email-masked]> writes:

> +static void parse_cmd_symref_update(struct ref_transaction *transaction,
> +				    const char *next, const char *end)
> +{
> +	char *refname, *new_target, *old_arg;
> +	char *old_target = NULL;
> + ...
> +	old_arg = parse_next_arg(&next);

> +	if (old_arg) {
> +		old_target = parse_next_arg(&next);

Now we have an allocated memory we are responsible for freeing in
old_target, obtained from parse_next_arg() ...

> +		if (!old_target)
> +			die("symref-update %s: expected old value", refname);

... and here we know it is not NULL.  We use it to grab the object
name ...

> +		if (!strcmp(old_arg, "oid")) {
> +			if (repo_get_oid(the_repository, old_target, &old_oid))
> +				die("symref-update %s: invalid oid: %s", refname, old_target);
> +
> +			old_target = NULL;

... and then we overwritten the variable, losing the last reference
to the piece of memory without freeing.

Perhaps squashing this in is sufficient to plug this leak, but there
probably are other new leaks around this code.  I ran out of time so
I'll let you take care of the rest ;-)

Thanks.

 builtin/update-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git c/builtin/update-ref.c w/builtin/update-ref.c
index 76d20ca0f1..7d2a419230 100644
--- c/builtin/update-ref.c
+++ w/builtin/update-ref.c
@@ -297,7 +297,7 @@ static void parse_cmd_symref_update(struct ref_transaction *transaction,
 
 		if (!strcmp(old_arg, "oid") &&
 		    !repo_get_oid(the_repository, old_target, &old_oid)) {
-			old_target = NULL;
+			FREE_AND_NULL(old_target);
 			have_old = 1;
 		} else if (strcmp(old_arg, "ref"))
 			die("symref-update %s: invalid arg '%s' for old value", refname, old_arg);