diff mbox series

[19/26] builtin/branch: fix leaking sorting options

Message ID e0aa2c5bb258589bb2cac19c54fddbb70614a487.1730901926.git.ps@pks.im (mailing list archive)
State New
Headers show
Series [01/26] builtin/blame: fix leaking blame entries with `--incremental` | expand

Commit Message

Patrick Steinhardt Nov. 6, 2024, 3:11 p.m. UTC 
The sorting options are leaking, but given that they are marked with
`UNLEAK()` the leak sanitizer doesn't complain.

Fix the leak by creating a common exit path and clearing the vector such
that we can get rid of the `UNLEAK()` annotation entirely.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/branch.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

Comments

Rubén Justo Nov. 10, 2024, 9:47 p.m. UTC | #1 
On Wed, Nov 06, 2024 at 04:11:16PM +0100, Patrick Steinhardt wrote:
> The sorting options are leaking, but given that they are marked with
> `UNLEAK()` the leak sanitizer doesn't complain.
> 
> Fix the leak by creating a common exit path and clearing the vector such
> that we can get rid of the `UNLEAK()` annotation entirely.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/branch.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/builtin/branch.c b/builtin/branch.c
> index fd1611ebf55..05ba4cd7a64 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -722,6 +722,7 @@ int cmd_branch(int argc,
>  	static struct ref_sorting *sorting;
>  	struct string_list sorting_options = STRING_LIST_INIT_DUP;
>  	struct ref_format format = REF_FORMAT_INIT;
> +	int ret;
>  
>  	struct option options[] = {
>  		OPT_GROUP(N_("Generic options")),
> @@ -851,15 +852,15 @@ int cmd_branch(int argc,
>  	if (list)
>  		setup_auto_pager("branch", 1);
>  
> -	UNLEAK(sorting_options);
> -
>  	if (delete) {
>  		if (!argc)
>  			die(_("branch name required"));
> -		return delete_branches(argc, argv, delete > 1, filter.kind, quiet);
> +		ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
> +		goto out;
>  	} else if (show_current) {
>  		print_current_branch_name();
> -		return 0;
> +		ret = 0;
> +		goto out;
>  	} else if (list) {
>  		/*  git branch --list also shows HEAD when it is detached */
>  		if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
> @@ -882,12 +883,13 @@ int cmd_branch(int argc,
>  		ref_sorting_release(sorting);
>  		ref_filter_clear(&filter);
>  		ref_format_clear(&format);
> -		return 0;
> +
> +		ret = 0;
> +		goto out;
>  	} else if (edit_description) {
>  		const char *branch_name;
>  		struct strbuf branch_ref = STRBUF_INIT;
>  		struct strbuf buf = STRBUF_INIT;
> -		int ret = 1; /* assume failure */

Now we assume failure ...

>  
>  		if (!argc) {
>  			if (filter.detached)
> @@ -901,18 +903,22 @@ int cmd_branch(int argc,
>  		}
>  
>  		strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
> -		if (!refs_ref_exists(get_main_ref_store(the_repository), branch_ref.buf))
> +		if (!refs_ref_exists(get_main_ref_store(the_repository), branch_ref.buf)) {
>  			error((!argc || branch_checked_out(branch_ref.buf))
>  			      ? _("no commit on branch '%s' yet")
>  			      : _("no branch named '%s'"),
>  			      branch_name);
> -		else if (!edit_branch_description(branch_name))
> +			ret = 1;
> +		} else if (!edit_branch_description(branch_name)) {
>  			ret = 0; /* happy */
> +		} else {
> +			ret = 1;

... here.  OK.

> +		}
>  
>  		strbuf_release(&branch_ref);
>  		strbuf_release(&buf);
>  
> -		return ret;
> +		goto out;
>  	} else if (copy || rename) {
>  		if (!argc)
>  			die(_("branch name required"));
> @@ -1000,12 +1006,17 @@ int cmd_branch(int argc,
>  			create_branches_recursively(the_repository, branch_name,
>  						    start_name, NULL, force,
>  						    reflog, quiet, track, 0);
> -			return 0;
> +			ret = 0;
> +			goto out;
>  		}
>  		create_branch(the_repository, branch_name, start_name, force, 0,
>  			      reflog, quiet, track, 0);
>  	} else
>  		usage_with_options(builtin_branch_usage, options);
>  
> -	return 0;
> +	ret = 0;
> +
> +out:
> +	string_list_clear(&sorting_options, 0);
> +	return ret;
>  }
> -- 
> 2.47.0.229.g8f8d6eee53.dirty
>
diff mbox series

Patch

diff --git a/builtin/branch.c b/builtin/branch.c
index fd1611ebf55..05ba4cd7a64 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -722,6 +722,7 @@  int cmd_branch(int argc,
 	static struct ref_sorting *sorting;
 	struct string_list sorting_options = STRING_LIST_INIT_DUP;
 	struct ref_format format = REF_FORMAT_INIT;
+	int ret;
 
 	struct option options[] = {
 		OPT_GROUP(N_("Generic options")),
@@ -851,15 +852,15 @@  int cmd_branch(int argc,
 	if (list)
 		setup_auto_pager("branch", 1);
 
-	UNLEAK(sorting_options);
-
 	if (delete) {
 		if (!argc)
 			die(_("branch name required"));
-		return delete_branches(argc, argv, delete > 1, filter.kind, quiet);
+		ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
+		goto out;
 	} else if (show_current) {
 		print_current_branch_name();
-		return 0;
+		ret = 0;
+		goto out;
 	} else if (list) {
 		/*  git branch --list also shows HEAD when it is detached */
 		if ((filter.kind & FILTER_REFS_BRANCHES) && filter.detached)
@@ -882,12 +883,13 @@  int cmd_branch(int argc,
 		ref_sorting_release(sorting);
 		ref_filter_clear(&filter);
 		ref_format_clear(&format);
-		return 0;
+
+		ret = 0;
+		goto out;
 	} else if (edit_description) {
 		const char *branch_name;
 		struct strbuf branch_ref = STRBUF_INIT;
 		struct strbuf buf = STRBUF_INIT;
-		int ret = 1; /* assume failure */
 
 		if (!argc) {
 			if (filter.detached)
@@ -901,18 +903,22 @@  int cmd_branch(int argc,
 		}
 
 		strbuf_addf(&branch_ref, "refs/heads/%s", branch_name);
-		if (!refs_ref_exists(get_main_ref_store(the_repository), branch_ref.buf))
+		if (!refs_ref_exists(get_main_ref_store(the_repository), branch_ref.buf)) {
 			error((!argc || branch_checked_out(branch_ref.buf))
 			      ? _("no commit on branch '%s' yet")
 			      : _("no branch named '%s'"),
 			      branch_name);
-		else if (!edit_branch_description(branch_name))
+			ret = 1;
+		} else if (!edit_branch_description(branch_name)) {
 			ret = 0; /* happy */
+		} else {
+			ret = 1;
+		}
 
 		strbuf_release(&branch_ref);
 		strbuf_release(&buf);
 
-		return ret;
+		goto out;
 	} else if (copy || rename) {
 		if (!argc)
 			die(_("branch name required"));
@@ -1000,12 +1006,17 @@  int cmd_branch(int argc,
 			create_branches_recursively(the_repository, branch_name,
 						    start_name, NULL, force,
 						    reflog, quiet, track, 0);
-			return 0;
+			ret = 0;
+			goto out;
 		}
 		create_branch(the_repository, branch_name, start_name, force, 0,
 			      reflog, quiet, track, 0);
 	} else
 		usage_with_options(builtin_branch_usage, options);
 
-	return 0;
+	ret = 0;
+
+out:
+	string_list_clear(&sorting_options, 0);
+	return ret;
 }