use strip_suffix and xstrfmt to replace suffix

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 <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/http.c b/http.c
index 7b0225996189fc5e860249b484947285a1281d63..e0ff876cd96c2ee6bdab82081370e091e8d311e2 100644 (file)
--- 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;
        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];
 
        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;
 
                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);

 
        ip_argv[0] = "index-pack";
        ip_argv[1] = "-o";
 
        ip_argv[0] = "index-pack";
        ip_argv[1] = "-o";

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 637770af813eee643c8375051e419715f56a2db5..7dfcb341d61aa93e2c655963571ba1809fc93d40 100644 (file)
--- 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)
 {
 
 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)
 }
 
 static int open_pack_bitmap_1(struct packed_git *packfile)

diff --git a/sha1_file.c b/sha1_file.c
index 592226eb7bb9e961b357b11f678773f7ac74ab6f..2be1afdde0b1bde19583ad8fa183fb7f2546cc13 100644 (file)
--- 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;
 int open_pack_index(struct packed_git *p)
 {
        char *idx_name;

+       size_t len;

        int ret;
 
        if (p->index_data)
                return 0;
 
        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;
        ret = check_packed_git_idx(idx_name, p);
        free(idx_name);
        return ret;