Re: [PATCH v2 6/6] update-ref: add support for 'symref-update' command
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 25, 2024, 23:00 UTC
- Message-ID
- <xmqqplt9h4yq.fsf@gitster.g>
- In-Reply-To
- <20240522090326.1268326-7-knayak@gitlab.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 7 quoted lines
> +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 ...
Show 5 quoted lines
> + 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);