Re: [PATCH 29/67] use strip_suffix and xstrfmt to replace suffix
flat view
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:06:34
On Tue, Sep 15, 2015 at 11:47 AM, Jeff King [off-list ref] wrote:
quoted hunk ↗ jump to hunk
When we want to convert "foo.pack" to "foo.idx", we do it by duplicating the original string and then munging the bytes in place. Let's use strip_suffix and xstrfmt instead, which has several advantages: 1. It's more clear what the intent is. 2. It does not implicitly rely on the fact that strlen(".idx") <= strlen(".pack") to avoid an overflow. 3. We communicate the assumption that the input file ends with ".pack" (and get a run-time check that this is so). 4. We drop calls to strcpy, which makes auditing the code base easier. Likewise, we can do this to convert ".pack" to ".bitmap", avoiding some manual memory computation. Signed-off-by: Jeff King <redacted> ---diff --git a/http.c b/http.c index 7b02259..e0ff876 100644 --- a/http.c +++ b/http.c@@ -1511,6 +1511,7 @@ int finish_http_pack_request(struct http_pack_request *preq) struct packed_git **lst; struct packed_git *p = preq->target; char *tmp_idx; + size_t len; struct child_process ip = CHILD_PROCESS_INIT; const char *ip_argv[8];@@ -1524,9 +1525,9 @@ int finish_http_pack_request(struct http_pack_request *preq) lst = &((*lst)->next); *lst = (*lst)->next; - tmp_idx = xstrdup(preq->tmpfile); - strcpy(tmp_idx + strlen(tmp_idx) - strlen(".pack.temp"), - ".idx.temp"); + if (!strip_suffix(preq->tmpfile, ".pack.temp", &len)) + die("BUG: pack tmpfile does not end in .pack.temp?"); + tmp_idx = xstrfmt("%.*s.idx.temp", (int)len, preq->tmpfile);
These instances of repeated replacement code may argue in favor of a
general purpose replace_suffix() function:
char *replace_suffix(const char *s, const char *old, const char *new)
{
size_t n;
if (!strip_suffix(s, old, &n))
die("BUG: '%s' does not end with '%s', s, old);
return xstrfmt("%.*s%s", (int)n, s, new);
}
or something.
quoted hunk ↗ jump to hunk
ip_argv[0] = "index-pack"; ip_argv[1] = "-o";diff --git a/pack-bitmap.c b/pack-bitmap.c index 637770a..7dfcb34 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c@@ -252,16 +252,11 @@ static int load_bitmap_entries_v1(struct bitmap_index *index) static char *pack_bitmap_filename(struct packed_git *p) { - char *idx_name; - int len; - - len = strlen(p->pack_name) - strlen(".pack"); - idx_name = xmalloc(len + strlen(".bitmap") + 1); - - memcpy(idx_name, p->pack_name, len); - memcpy(idx_name + len, ".bitmap", strlen(".bitmap") + 1); + size_t len; - return idx_name; + if (!strip_suffix(p->pack_name, ".pack", &len)) + die("BUG: pack_name does not end in .pack"); + return xstrfmt("%.*s.bitmap", (int)len, p->pack_name); } static int open_pack_bitmap_1(struct packed_git *packfile)diff --git a/sha1_file.c b/sha1_file.c index 28352a5..88996f0 100644 --- a/sha1_file.c +++ b/sha1_file.c@@ -671,13 +671,15 @@ static int check_packed_git_idx(const char *path, struct packed_git *p) int open_pack_index(struct packed_git *p) { char *idx_name; + size_t len; int ret; if (p->index_data) return 0; - idx_name = xstrdup(p->pack_name); - strcpy(idx_name + strlen(idx_name) - strlen(".pack"), ".idx"); + if (!strip_suffix(p->pack_name, ".pack", &len)) + die("BUG: pack_name does not end in .pack"); + idx_name = xstrfmt("%.*s.idx", (int)len, p->pack_name); ret = check_packed_git_idx(idx_name, p); free(idx_name); return ret; --2.6.0.rc2.408.ga2926b9