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

Re: [PATCH 1/2] stop calling UNLEAK() before die()

From
Derrick Stolee <stolee@gmail.com>
Date
Aug 13, 2020, 18:08 UTC
Message-ID
<275f3f6c-77ed-fddd-8f79-28f25b257362@gmail.com>
In-Reply-To
<20200813155500.GA897132@coredump.intra.peff.net>
On 8/13/2020 11:55 AM, Jeff King wrote:
Show 8 quoted lines
> The point of UNLEAK() is to make a reference to a variable that is about
> to go out of scope so that leak-checkers will consider it to be
> not-leaked. Doing so right before die() is therefore pointless; even
> though we are about to exit the program, the variable will still be on
> the stack and accessible to leak-checkers.
> 
> These annotations aren't really hurting anything, but they clutter the
> code and set a bad example of how to use UNLEAK().
Good justification. I'll stop being a bad example ;)

Thanks, -Stolee

Show 52 quoted lines
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  bugreport.c | 4 +---
>  midx.c      | 8 ++------
>  2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/bugreport.c b/bugreport.c
> index 09579e268d..7ca0fba1b8 100644
> --- a/bugreport.c
> +++ b/bugreport.c
> @@ -175,10 +175,8 @@ int cmd_main(int argc, const char **argv)
>  	/* fopen doesn't offer us an O_EXCL alternative, except with glibc. */
>  	report = open(report_path.buf, O_CREAT | O_EXCL | O_WRONLY, 0666);
>  
> -	if (report < 0) {
> -		UNLEAK(report_path);
> +	if (report < 0)
>  		die(_("couldn't create a new file at '%s'"), report_path.buf);
> -	}
>  
>  	if (write_in_full(report, buffer.buf, buffer.len) < 0)
>  		die_errno(_("unable to write to %s"), report_path.buf);
> diff --git a/midx.c b/midx.c
> index a5fb797ede..737420f157 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -807,11 +807,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *
>  	int result = 0;
>  
>  	midx_name = get_midx_filename(object_dir);
> -	if (safe_create_leading_directories(midx_name)) {
> -		UNLEAK(midx_name);
> +	if (safe_create_leading_directories(midx_name))
>  		die_errno(_("unable to create leading directories of %s"),
>  			  midx_name);
> -	}
>  
>  	if (m)
>  		packs.m = m;
> @@ -1051,10 +1049,8 @@ void clear_midx_file(struct repository *r)
>  		r->objects->multi_pack_index = NULL;
>  	}
>  
> -	if (remove_path(midx)) {
> -		UNLEAK(midx);
> +	if (remove_path(midx))
>  		die(_("failed to clear multi-pack-index at %s"), midx);
> -	}
>  
>  	free(midx);
>  }
> 
Previous: Jeff KingNext: Jeff King
Message 3 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.