From: Jeff King <peff@peff.net>
Date: Fri, 29 Jan 2021 00:28:32 GMT
Subject: Re: [PATCH v3 03/10] builtin/index-pack.c: allow stripping arbitrary extensions
Message-ID: <YBNWsJBDUpNiRXP4@coredump.intra.peff.net>
In-Reply-To: <8a3e70454b9bc64fc7a5ff07d47f7fde018e6a3d.1611617820.git.me@ttaylorr.com>

On Mon, Jan 25, 2021 at 06:37:22PM -0500, Taylor Blau wrote:

> To derive the filename for a .idx file, 'git index-pack' uses
> derive_filename() to strip the '.pack' suffix and add the new suffix.
> 
> Prepare for stripping off suffixes other than '.pack' by making the
> suffix to strip a parameter of derive_filename(). In order to make this
> consistent with the "suffix" parameter which does not begin with a ".",
> an additional check in derive_filename.

Maybe "add" missing from the final line?

> +static const char *derive_filename(const char *pack_name, const char *strip,
> +				   const char *suffix, struct strbuf *buf)
>  {
>  	size_t len;
> -	if (!strip_suffix(pack_name, ".pack", &len))
> -		die(_("packfile name '%s' does not end with '.pack'"),
> -		    pack_name);
> +	if (!strip_suffix(pack_name, strip, &len) || !len ||
> +	    pack_name[len - 1] != '.')
> +		die(_("packfile name '%s' does not end with '.%s'"),
> +		    pack_name, strip);
>  	strbuf_add(buf, pack_name, len);
> -	strbuf_addch(buf, '.');

Looks good to me.

-Peff