add_packed_git: convert strcpy into xsnprintf

We have the path "foo.idx", and we create a buffer big enough to hold "foo.pack" and "foo.keep", and then strcpy straight into it. This isn't a bug (we have enough space), but it's very hard to tell from the strcpy that this is so. Let's instead use strip_suffix to take off the ".idx", record the size of our allocation, and use xsnprintf to make sure we don't violate our assumptions. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 24, 2015 at 17:06 UTC 48bcc1c3cc09db1a6da0ce47460fae6e5f7edd4b
2 files changed +14 -9
cache.h
+1 -1
@@ -1309,7 +1309,7 @@ extern void close_pack_windows(struct packed_git *);
1309 extern void unuse_pack(struct pack_window **);
1310 extern void free_pack_by_name(const char *);
1311 extern void clear_delta_base_cache(void);
1312 -extern struct packed_git *add_packed_git(const char *, int, int);
1312 +extern struct packed_git *add_packed_git(const char *path, size_t path_len, int local);
1313
1314 /*
1315 * Return the SHA-1 of the nth object within the specified packfile.
sha1_file.c
+13 -8
@@ -1146,11 +1146,12 @@ static void try_to_free_pack_memory(size_t size)
1146 release_pack_memory(size);
1147 }
1148
1149 -struct packed_git *add_packed_git(const char *path, int path_len, int local)
1149 +struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
1150 {
1151 static int have_set_try_to_free_routine;
1152 struct stat st;
1153 - struct packed_git *p = alloc_packed_git(path_len + 2);
1153 + size_t alloc;
1154 + struct packed_git *p;
1155
1156 if (!have_set_try_to_free_routine) {
1157 have_set_try_to_free_routine = 1;
@@ -1161,18 +1162,22 @@ struct packed_git *add_packed_git(const char *path, int path_len, int local)
1162 * Make sure a corresponding .pack file exists and that
1163 * the index looks sane.
1164 */
1164 - path_len -= strlen(".idx");
1165 - if (path_len < 1) {
1166 - free(p);
1165 + if (!strip_suffix_mem(path, &path_len, ".idx"))
1166 return NULL;
1168 - }
1167 +
1168 + /*
1169 + * ".pack" is long enough to hold any suffix we're adding (and
1170 + * the use xsnprintf double-checks that)
1171 + */
1172 + alloc = path_len + strlen(".pack") + 1;
1173 + p = alloc_packed_git(alloc);
1174 memcpy(p->pack_name, path, path_len);
1175
1171 - strcpy(p->pack_name + path_len, ".keep");
1176 + xsnprintf(p->pack_name + path_len, alloc - path_len, ".keep");
1177 if (!access(p->pack_name, F_OK))
1178 p->pack_keep = 1;
1179
1175 - strcpy(p->pack_name + path_len, ".pack");
1180 + xsnprintf(p->pack_name + path_len, alloc - path_len, ".pack");
1181 if (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {
1182 free(p);
1183 return NULL;