Re: [PATCH v4] setup: don't die if realpath(3) fails on getcwd(3)
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 24, 2022, 21:25 UTC
- Message-ID
- <xmqqk0aafwsd.fsf@gitster.g>
- In-Reply-To
- <8b20840014d214023c50ee62439147f798e6f9cc.1653419993.git.kevin@kevinlocke.name>
Kevin Locke <kevin@kevinlocke.name> writes:
Show 15 quoted lines
> /* Normalize the directory */
> - strbuf_realpath(&tmp, tmp_original_cwd, 1);
> + if (!strbuf_realpath(&tmp, tmp_original_cwd, 0)) {
> + trace2_data_string("setup", the_repository,
> + "realpath-path", tmp_original_cwd);
> + trace2_data_string("setup", the_repository,
> + "realpath-failure", strerror(errno));
> + free((char*)tmp_original_cwd);
> + tmp_original_cwd = NULL;
> + return;
> + }
> +
> free((char*)tmp_original_cwd);
> tmp_original_cwd = NULL;
> startup_info->original_cwd = strbuf_detach(&tmp, NULL);It is somewhat sad that we cannot readily use FREE_AND_NULL() here. If it casted away the constness (see the attached at the end), we could have saved two lines from the above snippet.
The startup_info->original_cwd member is initialized to NULL, and I think it is a safe assumption that it still is so when the control reaches here. Otherwise, the assignment of strbuf_detach() to it without first freeing the current value we see in the post context is leaking already.
So, overall this looks good to me. Anybody else who have already spent cycles to review this want to add Reviewed-by: to it?
Thanks.
diff --git i/git-compat-util.h w/git-compat-util.h index 58fd813bd0..56c6c48461 100644 --- i/git-compat-util.h +++ w/git-compat-util.h @@ -976,7 +976,7 @@ int xstrncmpz(const char *s, const char *t, size_t len); * FREE_AND_NULL(ptr) is like free(ptr) followed by ptr = NULL. Note * that ptr is used twice, so don't pass e.g. ptr++. */ -#define FREE_AND_NULL(p) do { free(p); (p) = NULL; } while (0) +#define FREE_AND_NULL(p) do { free((void*)p); (p) = NULL; } while (0) #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc))) #define CALLOC_ARRAY(x, alloc) (x) = xcalloc((alloc), sizeof(*(x)))