Skip to content

Commit 27d69a4

Browse files
Nicolas Pitregitster
authored andcommitted
refactor pack structure allocation
New pack structures are currently allocated in 2 different places and all members have to be initialized explicitly. This is prone to errors leading to segmentation faults as found by Teemu Likonen. Let's have a common place where this structure is allocated, and have all members explicitly initialized to zero. Signed-off-by: Nicolas Pitre <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 29b0d01 commit 27d69a4

File tree

1 file changed

+15
-18
lines changed

1 file changed

+15
-18
lines changed

sha1_file.c

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -792,18 +792,28 @@ unsigned char* use_pack(struct packed_git *p,
792792
return win->base + offset;
793793
}
794794

795+
static struct packed_git *alloc_packed_git(int extra)
796+
{
797+
struct packed_git *p = xmalloc(sizeof(*p) + extra);
798+
memset(p, 0, sizeof(*p));
799+
p->pack_fd = -1;
800+
return p;
801+
}
802+
795803
struct packed_git *add_packed_git(const char *path, int path_len, int local)
796804
{
797805
struct stat st;
798-
struct packed_git *p = xmalloc(sizeof(*p) + path_len + 2);
806+
struct packed_git *p = alloc_packed_git(path_len + 2);
799807

800808
/*
801809
* Make sure a corresponding .pack file exists and that
802810
* the index looks sane.
803811
*/
804812
path_len -= strlen(".idx");
805-
if (path_len < 1)
813+
if (path_len < 1) {
814+
free(p);
806815
return NULL;
816+
}
807817
memcpy(p->pack_name, path, path_len);
808818
strcpy(p->pack_name + path_len, ".pack");
809819
if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {
@@ -814,16 +824,7 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)
814824
/* ok, it looks sane as far as we can check without
815825
* actually mapping the pack file.
816826
*/
817-
p->index_version = 0;
818-
p->index_data = NULL;
819-
p->index_size = 0;
820-
p->num_objects = 0;
821-
p->num_bad_objects = 0;
822-
p->bad_object_sha1 = NULL;
823827
p->pack_size = st.st_size;
824-
p->next = NULL;
825-
p->windows = NULL;
826-
p->pack_fd = -1;
827828
p->pack_local = local;
828829
p->mtime = st.st_mtime;
829830
if (path_len < 40 || get_sha1_hex(path + path_len - 40, p->sha1))
@@ -835,19 +836,15 @@ struct packed_git *parse_pack_index(unsigned char *sha1)
835836
{
836837
const char *idx_path = sha1_pack_index_name(sha1);
837838
const char *path = sha1_pack_name(sha1);
838-
struct packed_git *p = xmalloc(sizeof(*p) + strlen(path) + 2);
839+
struct packed_git *p = alloc_packed_git(strlen(path) + 1);
839840

841+
strcpy(p->pack_name, path);
842+
hashcpy(p->sha1, sha1);
840843
if (check_packed_git_idx(idx_path, p)) {
841844
free(p);
842845
return NULL;
843846
}
844847

845-
strcpy(p->pack_name, path);
846-
p->pack_size = 0;
847-
p->next = NULL;
848-
p->windows = NULL;
849-
p->pack_fd = -1;
850-
hashcpy(p->sha1, sha1);
851848
return p;
852849
}
853850

0 commit comments

Comments
 (0)