odb: fix subtle logic to check whether an alternate is usable

When adding an alternate to the object database we first check whether or not the path is usable. A path is usable if: - It actually exists. - We don't have it in our object sources yet. While the former check is trivial enough, the latter part is somewhat subtle and prone for bugs. This is because the function doesn't only check whether or not the given path is usable. But if it _is_ usable, we also store that path in the map of object sources immediately. The tricky part here is that the path that gets stored in the map is _not_ copied. Instead, we rely on the fact that subsequent code uses `strbuf_detach()` to store the exact same allocated memory in the created object source. Consequently, the memory is owned by the source but _also_ stored in the map. This subtlety is easy to miss, so if one decides to refactor this code one can easily end up breaking this mechanism. Make the relationship more explicit by not storing the path as part of `alt_odb_usable()`. Instead, store the path after we have created the source so that we can use the source's path pointer directly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Nov 3, 2025 at 08:41 UTC f82e430b4e46120e0ef67959e0ef9d8ab9282c56
1 file changed +17 -13
odb.c
+17 -13
@@ -86,17 +86,16 @@ int odb_mkstemp(struct object_database *odb,
86 /*
87 * Return non-zero iff the path is usable as an alternate object database.
88 */
89 -static int alt_odb_usable(struct object_database *o,
90 - struct strbuf *path,
91 - const char *normalized_objdir, khiter_t *pos)
89 +static int alt_odb_usable(struct object_database *o, const char *path,
90 + const char *normalized_objdir)
91 {
92 int r;
93
94 /* Detect cases where alternate disappeared */
96 - if (!is_directory(path->buf)) {
95 + if (!is_directory(path)) {
96 error(_("object directory %s does not exist; "
97 "check .git/objects/info/alternates"),
99 - path->buf);
98 + path);
99 return 0;
100 }
101
@@ -113,11 +112,14 @@ static int alt_odb_usable(struct object_database *o,
112 assert(r == 1); /* never used */
113 kh_value(o->source_by_path, p) = o->sources;
114 }
116 - if (fspatheq(path->buf, normalized_objdir))
115 +
116 + if (fspatheq(path, normalized_objdir))
117 + return 0;
118 +
119 + if (kh_get_odb_path_map(o->source_by_path, path) < kh_end(o->source_by_path))
120 return 0;
118 - *pos = kh_put_odb_path_map(o->source_by_path, path->buf, &r);
119 - /* r: 0 = exists, 1 = never used, 2 = deleted */
120 - return r == 0 ? 0 : 1;
121 +
122 + return 1;
123 }
124
125 /*
@@ -148,6 +150,7 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
150 struct strbuf pathbuf = STRBUF_INIT;
151 struct strbuf tmp = STRBUF_INIT;
152 khiter_t pos;
153 + int ret;
154
155 if (!is_absolute_path(dir) && relative_base) {
156 strbuf_realpath(&pathbuf, relative_base, 1);
@@ -172,20 +175,21 @@ static struct odb_source *link_alt_odb_entry(struct object_database *odb,
175 strbuf_reset(&tmp);
176 strbuf_realpath(&tmp, odb->sources->path, 1);
177
175 - if (!alt_odb_usable(odb, &pathbuf, tmp.buf, &pos))
178 + if (!alt_odb_usable(odb, pathbuf.buf, tmp.buf))
179 goto error;
180
181 CALLOC_ARRAY(alternate, 1);
182 alternate->odb = odb;
183 alternate->local = false;
181 - /* pathbuf.buf is already in r->objects->source_by_path */
184 alternate->path = strbuf_detach(&pathbuf, NULL);
185
186 /* add the alternate entry */
187 *odb->sources_tail = alternate;
188 odb->sources_tail = &(alternate->next);
187 - alternate->next = NULL;
188 - assert(odb->source_by_path);
189 +
190 + pos = kh_put_odb_path_map(odb->source_by_path, alternate->path, &ret);
191 + if (!ret)
192 + BUG("source must not yet exist");
193 kh_value(odb->source_by_path, pos) = alternate;
194
195 /* recursively add alternates */