Re: [PATCH v2 2/5] sha1_file.c: make pack-name helper globally accessible

From: Jeff King <peff@peff.net>
Date: Mar 16, 2017, 14:31 UTC
Message-ID: <20170316143152.gdeaprlsercvwmiw@sigill.intra.peff.net>
In-Reply-To: <20170316142706.kt56a43phiuhhra7@sigill.intra.peff.net>

On Thu, Mar 16, 2017 at 10:27:06AM -0400, Jeff King wrote:

Show 18 quoted lines

> -/*
> - * Return the name of the pack or index file with the specified sha1
> - * in its filename.  *base and *name are scratch space that must be
> - * provided by the caller.  which should be "pack" or "idx".
> - */
> -static char *sha1_get_pack_name(const unsigned char *sha1,
> -				struct strbuf *buf,
> -				const char *which)
> + char *odb_pack_name(struct strbuf *buf,
> +		     const unsigned char *sha1,
> +		     const char *ext)
>  {
>  	strbuf_reset(buf);
>  	strbuf_addf(buf, "%s/pack/pack-%s.%s", get_object_directory(),
> -		    sha1_to_hex(sha1), which);
> +		    sha1_to_hex(sha1), ext);
>  	return buf->buf;
>  }

Incidentally, this entire function could be implemented as:

  return git_path_buf(buf, "objects/pack/pack-%s.%s",
                      sha1_to_hex(sha1), ext);

as the git_path() functions are smart enough to replace "objects/" with the true object directory when necessary. I don't know if people find that more or less readable. Since it's buried in a helper function, I doubt it matters much either way. The git_path functions do also do some path normalization, which might be of value.

-Peff