git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 2/2] ls-remote: simplify UNLEAK() usage

From
Derrick Stolee <stolee@gmail.com>
Date
Aug 13, 2020, 18:11 UTC
Message-ID
<df89c33b-87c7-8d1e-501f-f0660aecbed0@gmail.com>
In-Reply-To
<20200813155551.GB897132@coredump.intra.peff.net>
On 8/13/2020 11:55 AM, Jeff King wrote:
Show 9 quoted lines
> 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
Show 47 quoted lines
> 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;
>  }
> 
Previous: Jeff KingNext: Eric Sunshine
Message 6 of 9 in “UNLEAK style fixes”
  1. 0/2 UNLEAK style fixesJeff King, Aug 13, 2020
  2. 1/2 stop calling UNLEAK() before die()Jeff King, Aug 13, 2020
  3. Derrick StoleeAug 13, 2020
  4. Jeff KingAug 14, 2020
  5. 2/2 ls-remote: simplify UNLEAK() usageJeff King, Aug 13, 2020
  6. Derrick StoleeAug 13, 2020
  7. Eric SunshineAug 13, 2020
  8. Jeff KingAug 14, 2020
  9. Eric SunshineAug 14, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.