| Message ID | 8a3e70454b9bc64fc7a5ff07d47f7fde018e6a3d.1611617820.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pack-revindex: introduce on-disk '.rev' format | expand |
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
On Thu, Jan 28, 2021 at 07:28:32PM -0500, Jeff King wrote: > On Mon, Jan 25, 2021 at 06:37:22PM -0500, Taylor Blau wrote: > > 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? Oops, yeah. I'm happy to send a replacement to help with queuing, if the maintainer thinks that would be helpful. Thanks, Taylor
diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 557bd2f348..c758f3b8e9 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1436,15 +1436,15 @@ static void fix_unresolved_deltas(struct hashfile *f) free(sorted_by_pos); } -static const char *derive_filename(const char *pack_name, const char *suffix, - struct strbuf *buf) +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, '.'); strbuf_addstr(buf, suffix); return buf->buf; } @@ -1459,7 +1459,7 @@ static void write_special_file(const char *suffix, const char *msg, int msg_len = strlen(msg); if (pack_name) - filename = derive_filename(pack_name, suffix, &name_buf); + filename = derive_filename(pack_name, "pack", suffix, &name_buf); else filename = odb_pack_name(&name_buf, hash, suffix); @@ -1824,7 +1824,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix) if (from_stdin && hash_algo) die(_("--object-format cannot be used with --stdin")); if (!index_name && pack_name) - index_name = derive_filename(pack_name, "idx", &index_name_buf); + index_name = derive_filename(pack_name, "pack", "idx", &index_name_buf); if (verify) { if (!index_name)
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. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> --- builtin/index-pack.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)