alternates: use a separate scratch space

The alternate_object_database struct uses a single buffer both for storing the path to the alternate, and as a scratch buffer for forming object names. This is efficient (since otherwise we'd end up storing the path twice), but it makes life hard for callers who just want to know the path to the alternate. They have to remember to stop reading after "alt->name - alt->base" bytes, and to subtract one for the trailing '/'. It would be much simpler if they could simply access a NUL-terminated path string. We could encapsulate this in a function which puts a NUL in the scratch buffer and returns the string, but that opens up questions about the lifetime of the result. The first time another caller uses the alternate, the scratch buffer may get other data tacked onto it. Let's instead just store the root path separately from the scratch buffer. There aren't enough alternates being stored for the duplicated data to matter for performance, and this keeps things simple and safe for the callers. 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:35 UTC 597f9134ded20f882e2bf6bca8b0e1f03981b98d
6 files changed +24 -37
builtin/fsck.c
+2 -8
@@ -644,14 +644,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
644 fsck_object_dir(get_object_directory());
645
646 prepare_alt_odb();
647 - for (alt = alt_odb_list; alt; alt = alt->next) {
648 - /* directory name, minus trailing slash */
649 - size_t namelen = alt->name - alt->base - 1;
650 - struct strbuf name = STRBUF_INIT;
651 - strbuf_add(&name, alt->base, namelen);
652 - fsck_object_dir(name.buf);
653 - strbuf_release(&name);
654 - }
647 + for (alt = alt_odb_list; alt; alt = alt->next)
648 + fsck_object_dir(alt->path);
649 }
650
651 if (check_full) {
builtin/submodule--helper.c
+3 -8
@@ -492,20 +492,16 @@ static int add_possible_reference_from_superproject(
492 {
493 struct submodule_alternate_setup *sas = sas_cb;
494
495 - /* directory name, minus trailing slash */
496 - size_t namelen = alt->name - alt->base - 1;
497 - struct strbuf name = STRBUF_INIT;
498 - strbuf_add(&name, alt->base, namelen);
499 -
495 /*
496 * If the alternate object store is another repository, try the
497 * standard layout with .git/modules/<name>/objects
498 */
504 - if (ends_with(name.buf, ".git/objects")) {
499 + if (ends_with(alt->path, ".git/objects")) {
500 char *sm_alternate;
501 struct strbuf sb = STRBUF_INIT;
502 struct strbuf err = STRBUF_INIT;
508 - strbuf_add(&sb, name.buf, name.len - strlen("objects"));
503 + strbuf_add(&sb, alt->path, strlen(alt->path) - strlen("objects"));
504 +
505 /*
506 * We need to end the new path with '/' to mark it as a dir,
507 * otherwise a submodule name containing '/' will be broken
@@ -533,7 +529,6 @@ static int add_possible_reference_from_superproject(
529 strbuf_release(&sb);
530 }
531
536 - strbuf_release(&name);
532 return 0;
533 }
534
cache.h
+4 -1
@@ -1383,8 +1383,11 @@ extern void remove_scheduled_dirs(void);
1383
1384 extern struct alternate_object_database {
1385 struct alternate_object_database *next;
1386 +
1387 char *name;
1387 - char base[FLEX_ARRAY]; /* more */
1388 + char *scratch;
1389 +
1390 + char path[FLEX_ARRAY];
1391 } *alt_odb_list;
1392 extern void prepare_alt_odb(void);
1393 extern void read_info_alternates(const char * relative_base, int depth);
sha1_file.c
+12 -16
@@ -208,7 +208,7 @@ static const char *alt_sha1_path(struct alternate_object_database *alt,
208 const unsigned char *sha1)
209 {
210 fill_sha1_path(alt->name, sha1);
211 - return alt->base;
211 + return alt->scratch;
212 }
213
214 /*
@@ -261,8 +261,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
261 * thing twice, or object directory itself.
262 */
263 for (alt = alt_odb_list; alt; alt = alt->next) {
264 - if (path->len == alt->name - alt->base - 1 &&
265 - !memcmp(path->buf, alt->base, path->len))
264 + if (!strcmp(path->buf, alt->path))
265 return 0;
266 }
267 if (!fspathcmp(path->buf, normalized_objdir))
@@ -401,13 +400,14 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
400 size_t entlen;
401
402 entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
404 - ent = xmalloc(st_add(sizeof(*ent), entlen));
405 - memcpy(ent->base, dir, dirlen);
403 + FLEX_ALLOC_STR(ent, path, dir);
404 + ent->scratch = xmalloc(entlen);
405 + xsnprintf(ent->scratch, entlen, "%s/", dir);
406
407 - ent->name = ent->base + dirlen + 1;
408 - ent->base[dirlen] = '/';
409 - ent->base[dirlen + 3] = '/';
410 - ent->base[entlen-1] = 0;
407 + ent->name = ent->scratch + dirlen + 1;
408 + ent->scratch[dirlen] = '/';
409 + ent->scratch[dirlen + 3] = '/';
410 + ent->scratch[entlen-1] = 0;
411
412 return ent;
413 }
@@ -1485,11 +1485,8 @@ void prepare_packed_git(void)
1485 return;
1486 prepare_packed_git_one(get_object_directory(), 1);
1487 prepare_alt_odb();
1488 - for (alt = alt_odb_list; alt; alt = alt->next) {
1489 - alt->name[-1] = 0;
1490 - prepare_packed_git_one(alt->base, 0);
1491 - alt->name[-1] = '/';
1492 - }
1488 + for (alt = alt_odb_list; alt; alt = alt->next)
1489 + prepare_packed_git_one(alt->path, 0);
1490 rearrange_packed_git();
1491 prepare_packed_git_mru();
1492 prepare_packed_git_run_once = 1;
@@ -3692,8 +3689,7 @@ static int loose_from_alt_odb(struct alternate_object_database *alt,
3689 struct strbuf buf = STRBUF_INIT;
3690 int r;
3691
3695 - /* copy base not including trailing '/' */
3696 - strbuf_add(&buf, alt->base, alt->name - alt->base - 1);
3692 + strbuf_addstr(&buf, alt->path);
3693 r = for_each_loose_file_in_objdir_buf(&buf,
3694 data->cb, NULL, NULL,
3695 data->data);
sha1_name.c
+2 -1
@@ -94,12 +94,13 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
94 for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
95 struct dirent *de;
96 DIR *dir;
97 +
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);
102 - dir = opendir(alt->base);
103 + dir = opendir(alt->scratch);
104 if (!dir)
105 continue;
106
transport.c
+1 -3
@@ -1084,9 +1084,7 @@ static int refs_from_alternate_cb(struct alternate_object_database *e,
1084 const struct ref *extra;
1085 struct alternate_refs_data *cb = data;
1086
1087 - e->name[-1] = '\0';
1088 - other = xstrdup(real_path(e->base));
1089 - e->name[-1] = '/';
1087 + other = xstrdup(real_path(e->path));
1088 len = strlen(other);
1089
1090 while (other[len-1] == '/')