Skip to content

Commit

Permalink
use strip_suffix and xstrfmt to replace suffix
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Sep 25, 2015
1 parent 2805bb5 commit 9ae9701
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 14 deletions.
7 changes: 4 additions & 3 deletions http.c
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand All @@ -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);

ip_argv[0] = "index-pack";
ip_argv[1] = "-o";
Expand Down
13 changes: 4 additions & 9 deletions pack-bitmap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions sha1_file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9ae9701

Please sign in to comment.