Subject: Re: [PATCH v3 2/3] builtin/commit: error out when passing untracked path with -i

From: Junio C Hamano <[email-masked]>
Date: Tue, 02 Apr 2024 14:47:02 -0700

Ghanshyam Thakkar <[email-masked]> writes:

> 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.