Re: [PATCH v3 2/3] builtin/commit: error out when passing untracked path with -i
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 2, 2024, 21:47 UTC
- Message-ID
- <xmqqmsqb30a1.fsf@gitster.g>
- In-Reply-To
- <20240402213640.139682-5-shyamthakkar001@gmail.com>
Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
Show 17 quoted lines
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 8f31decc6b..09c48a835a 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -441,10 +441,17 @@ static const char *prepare_index(const char **argv, const char *prefix,
> * (B) on failure, rollback the real index.
> */
> if (all || (also && pathspec.nr)) {
> + char *ps_matched = xcalloc(pathspec.nr, 1);
> repo_hold_locked_index(the_repository, &index_lock,
> LOCK_DIE_ON_ERROR);
> add_files_to_cache(the_repository, also ? prefix : NULL,
> - &pathspec, NULL, 0, 0);
> + &pathspec, ps_matched, 0, 0);
> + if (!all && report_path_error(ps_matched, &pathspec)) {
> + free(ps_matched);
> + exit(1);No need to free(ps_matched) immediately before exiting. There are other recources (like pathspec) we are holding and not clearing, and we do not want to bother cleaning them all.
As we have another "if failed, die()" immediately after this hunk, adding another exit() would be OK. Shouldn't we be exiting with 128 to match what die() does, though?
Other than that, looking good.