[2/2] ls-remote: simplify UNLEAK() usage
diff mbox series

Message ID 20200813155551.GB897132@coredump.intra.peff.net
State New
Headers show
Series
  • UNLEAK style fixes
Related show

Commit Message

Jeff King Aug. 13, 2020, 3:55 p.m. UTC
We UNLEAK() the "sorting" list created by parsing command-line options
(which is essentially used until the program exits). But we do so right
before leaving the cmd_ls_remote() function, which means we have to hit
all of the exits. But the point of UNLEAK() is that it's an annotation
which doesn't impact the variable itself. We can mark it as soon as
we're done writing its value, and then we only have to do so once.

This gives us a minor code reduction, and serves as a better example of
how UNLEAK() can be used.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/ls-remote.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

Comments

Derrick Stolee Aug. 13, 2020, 6:11 p.m. UTC | #1
On 8/13/2020 11:55 AM, Jeff King wrote:
> We UNLEAK() the "sorting" list created by parsing command-line options
> (which is essentially used until the program exits). But we do so right
> before leaving the cmd_ls_remote() function, which means we have to hit
> all of the exits. But the point of UNLEAK() is that it's an annotation
> which doesn't impact the variable itself. We can mark it as soon as
> we're done writing its value, and then we only have to do so once.
> 
> This gives us a minor code reduction, and serves as a better example of
> how UNLEAK() can be used.

LGTM. In hindsight, it's obvious that UNLEAK() can be called before we are
done using a variable. Otherwise, we'd just free() it instead.

-Stolee

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/ls-remote.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index ea91679f33..092917eca2 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -83,6 +83,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  			     PARSE_OPT_STOP_AT_NON_OPTION);
>  	dest = argv[0];
>  
> +	UNLEAK(sorting);
> +
>  	if (argc > 1) {
>  		int i;
>  		pattern = xcalloc(argc, sizeof(const char *));
> @@ -107,7 +109,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  
>  	if (get_url) {
>  		printf("%s\n", *remote->url);
> -		UNLEAK(sorting);
>  		return 0;
>  	}
>  
> @@ -122,10 +123,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  		int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
>  		repo_set_hash_algo(the_repository, hash_algo);
>  	}
> -	if (transport_disconnect(transport)) {
> -		UNLEAK(sorting);
> +	if (transport_disconnect(transport))
>  		return 1;
> -	}
>  
>  	if (!dest && !quiet)
>  		fprintf(stderr, "From %s\n", *remote->url);
> @@ -150,7 +149,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>  		status = 0; /* we found something */
>  	}
>  
> -	UNLEAK(sorting);
>  	ref_array_clear(&ref_array);
>  	return status;
>  }
>

Patch
diff mbox series

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index ea91679f33..092917eca2 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -83,6 +83,8 @@  int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	dest = argv[0];
 
+	UNLEAK(sorting);
+
 	if (argc > 1) {
 		int i;
 		pattern = xcalloc(argc, sizeof(const char *));
@@ -107,7 +109,6 @@  int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 
 	if (get_url) {
 		printf("%s\n", *remote->url);
-		UNLEAK(sorting);
 		return 0;
 	}
 
@@ -122,10 +123,8 @@  int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		int hash_algo = hash_algo_by_ptr(transport_get_hash_algo(transport));
 		repo_set_hash_algo(the_repository, hash_algo);
 	}
-	if (transport_disconnect(transport)) {
-		UNLEAK(sorting);
+	if (transport_disconnect(transport))
 		return 1;
-	}
 
 	if (!dest && !quiet)
 		fprintf(stderr, "From %s\n", *remote->url);
@@ -150,7 +149,6 @@  int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		status = 0; /* we found something */
 	}
 
-	UNLEAK(sorting);
 	ref_array_clear(&ref_array);
 	return status;
 }