diff mbox series

[2/2] Fix mem leak in branch.c due to not-free newly added virtual_name variable

Message ID 27f27f3afd76fc974350c0c94e20307879eead84.1679478126.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series branch: improve error log on branch not found by checking remotes refs | expand

Commit Message

Clement Mabileau March 22, 2023, 9:42 a.m. UTC 
From: ctmbl <mabileau.clement@gmail.com>

Previous commit introduced virtual_name variable which is the name of
the branch in case it has been a remote ref (used to check whether the
user simply forgot `--remote` flag) but didn't free it.
Call FREE_AND_NULL(virtual_name) to solve it.

Signed-off-by: Clement Mabileau <mabileau.clement@gmail.com>
---
 builtin/branch.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Junio C Hamano March 22, 2023, 4:52 p.m. UTC | #1 
"ctmbl via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: ctmbl <mabileau.clement@gmail.com>
>
> Previous commit introduced virtual_name variable which is the name of
> the branch in case it has been a remote ref (used to check whether the
> user simply forgot `--remote` flag) but didn't free it.
> Call FREE_AND_NULL(virtual_name) to solve it.

Do not introduce a bug in 1/2 and fix it in 2/2, unless it is
absolutely necessary.

Squash this one into the previous commit, before sending your
patches out to the list.  The public do not have to learn your
earlier mistakes, and before sending your patches out is the perfect
chance for you to pretend to be a person that is more perfect than
you are ;-).

Thanks.
Clement Mabileau March 22, 2023, 8 p.m. UTC | #2 
On 22/03/2023 17:52, Junio C Hamano wrote:
> "ctmbl via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: ctmbl <mabileau.clement@gmail.com>
>>
>> Previous commit introduced virtual_name variable which is the name of
>> the branch in case it has been a remote ref (used to check whether the
>> user simply forgot `--remote` flag) but didn't free it.
>> Call FREE_AND_NULL(virtual_name) to solve it.
> 
> Do not introduce a bug in 1/2 and fix it in 2/2, unless it is
> absolutely necessary.

I didn't know this practice, thanks for letting me know :-)

> Squash this one into the previous commit, before sending your
> patches out to the list.  The public do not have to learn your
> earlier mistakes, and before sending your patches out is the perfect
> chance for you to pretend to be a person that is more perfect than
> you are ;-).

I just did it!

Thanks for the review.
Junio C Hamano March 22, 2023, 8:52 p.m. UTC | #3 
Clement Mabileau <mabileau.clement@gmail.com> writes:

> I just did it!

Nice to hear.

> Thanks for the review.

Well, I didn't "review" anything, though.  I just commented on 2/2
that will become moot in your updated version, and I didn't even
look at 1/2.  

The change(s) as a whole need to be reviewed by somebody.

Thanks.
diff mbox series

Patch

diff --git a/builtin/branch.c b/builtin/branch.c
index 8e768761b9b..697636e2874 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -279,6 +279,7 @@  static int delete_branches(int argc, const char **argv, int force, int kinds,
 							| RESOLVE_REF_NO_RECURSE
 							| RESOLVE_REF_ALLOW_BAD_NAME,
 							&oid, &flags);
+				FREE_AND_NULL(virtual_name);
 				if (virtual_target)
 					error(_(MISSING_BRANCH_HINT_MSG), bname.buf);
 				else