alternates: store scratch buffer as strbuf
We pre-size the scratch buffer to hold a loose object filename of the form "xx/yyyy...", which leads to allocation code that is hard to verify. We have to use some magic numbers during the initial allocation, and then writers must blindly assume that the buffer is big enough. Using a strbuf makes it more clear that we cannot overflow. Unfortunately, we do still need some magic numbers to grow our strbuf before calling fill_sha1_path(), but the strbuf growth is much closer to the point of use. This makes it easier to see that it's correct, and opens the possibility of pushing it even further down if fill_sha1_path() learns to work on strbufs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Oct 3, 2016 at 16:36 UTC
38dbe5f07837afceaec95fae5981d36eeb4917bd
3 files changed
+32
-18
cache.h
+11
-2
@@ -1384,8 +1384,9 @@ extern void remove_scheduled_dirs(void);
1384
extern struct alternate_object_database {
1385
struct alternate_object_database *next;
1386
1387
- char *name;
1388
- char *scratch;
1387
+ /* see alt_scratch_buf() */
1388
+ struct strbuf scratch;
1389
+ size_t base_len;
1390
1391
char path[FLEX_ARRAY];
1392
} *alt_odb_list;
@@ -1414,6 +1415,14 @@ extern void add_to_alternates_file(const char *dir);
1415
*/
1416
extern void add_to_alternates_memory(const char *dir);
1417
1418
+/*
1419
+ * Returns a scratch strbuf pre-filled with the alternate object directory,
1420
+ * including a trailing slash, which can be used to access paths in the
1421
+ * alternate. Always use this over direct access to alt->scratch, as it
1422
+ * cleans up any previous use of the scratch buffer.
1423
+ */
1424
+extern struct strbuf *alt_scratch_buf(struct alternate_object_database *alt);
1425
+
1426
struct pack_window {
1427
struct pack_window *next;
1428
unsigned char *base;
sha1_file.c
+18
-10
@@ -204,11 +204,24 @@ const char *sha1_file_name(const unsigned char *sha1)
204
return buf;
205
}
206
207
+struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
208
+{
209
+ strbuf_setlen(&alt->scratch, alt->base_len);
210
+ return &alt->scratch;
211
+}
212
+
213
static const char *alt_sha1_path(struct alternate_object_database *alt,
214
const unsigned char *sha1)
215
{
210
- fill_sha1_path(alt->name, sha1);
211
- return alt->scratch;
216
+ /* hex sha1 plus internal "/" */
217
+ size_t len = GIT_SHA1_HEXSZ + 1;
218
+ struct strbuf *buf = alt_scratch_buf(alt);
219
+
220
+ strbuf_grow(buf, len);
221
+ fill_sha1_path(buf->buf + buf->len, sha1);
222
+ strbuf_setlen(buf, buf->len + len);
223
+
224
+ return buf->buf;
225
}
226
227
/*
@@ -396,16 +409,11 @@ void read_info_alternates(const char * relative_base, int depth)
409
struct alternate_object_database *alloc_alt_odb(const char *dir)
410
{
411
struct alternate_object_database *ent;
399
- size_t dirlen = strlen(dir);
400
- size_t entlen;
412
402
- entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
413
FLEX_ALLOC_STR(ent, path, dir);
404
- ent->scratch = xmalloc(entlen);
405
- xsnprintf(ent->scratch, entlen, "%s/", dir);
406
-
407
- ent->name = ent->scratch + dirlen + 1;
408
- ent->scratch[dirlen] = '/';
414
+ strbuf_init(&ent->scratch, 0);
415
+ strbuf_addf(&ent->scratch, "%s/", dir);
416
+ ent->base_len = ent->scratch.len;
417
418
return ent;
419
}
sha1_name.c
+3
-6
@@ -92,15 +92,12 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
92
93
xsnprintf(hex, sizeof(hex), "%.2s", hex_pfx);
94
for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
95
+ struct strbuf *buf = alt_scratch_buf(alt);
96
struct dirent *de;
97
DIR *dir;
98
98
- /*
99
- * every alt_odb struct has 42 extra bytes after the base
100
- * for exactly this purpose
101
- */
102
- xsnprintf(alt->name, 42, "%.2s/", hex_pfx);
103
- dir = opendir(alt->scratch);
99
+ strbuf_addf(buf, "%.2s/", hex_pfx);
100
+ dir = opendir(buf->buf);
101
if (!dir)
102
continue;
103