On 16/03/17 14:31, Jeff King wrote: > On Thu, Mar 16, 2017 at 10:27:06AM -0400, Jeff King wrote: > >> -/* >> - * 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.
Hmm, I don't have strong feelings either way. However, I note that the only normalization going on (that I can see) is to remove .//* from the beginning of the resulting string. I don't know why, but I guess it is to cater to people using the various GIT_ environment variables doing things like: $ GIT_OBJECT_DIRECTORY=./my-objects git .... It has always puzzled me slightly, why the git_path functions do this normalization, but (for example) setup_git_env(), git_path_from_env(), get_common_dir(), ... don't! ;-) ATB, Ramsay Jones