Am 03.07.21 um 12:05 schrieb René Scharfe:
> Am 29.06.21 um 22:53 schrieb Eric Wong:
>> + *pos = kh_put_odb_path_map(o->odb_by_path, path->buf, &r);
>> + if (r < 0) die_errno(_("kh_put_odb_path_map"));
>> + /* r: 0 = exists, 1 = never used, 2 = deleted */
>> + return r == 0 ? 0 : 1;
> I like the solution in oidset.c to make this more readable, though: Call
> the return value "added" instead of "r" and then a "return !added;"
> makes sense without additional comments.
That's probably because I wrote that part; see 8b2f8cbcb1 (oidset: use
khash, 2018-10-04) -- I had somehow forgotten about that. o_O
And here we wouldn't negate. Passing on the value verbatim, without
normalizing 2 to 1, would work fine.
alt_odb_usable() and its caller become quite entangled due to the
hashmap insert operation being split between them. I suspect the code
would improve by inlining the function in a follow-up patch, making
return code considerations moot. The improvement is not significant
enough to hold up this series in case you don't like the idea, though.
Rough demo:
object-file.c | 82 +++++++++++++++++++++++++++--------------------------------
1 file changed, 37 insertions(+), 45 deletions(-)
diff --git a/object-file.c b/object-file.c
index 304af3a172..a5e91091ee 100644
--- a/object-file.c
+++ b/object-file.c
@@ -512,45 +512,6 @@ const char *loose_object_path(struct repository *r, struct strbuf *buf,
return odb_loose_path(r->objects->odb, buf, oid);
}
-/*
- * Return non-zero iff the path is usable as an alternate object database.
- */
-static int alt_odb_usable(struct raw_object_store *o,
- struct strbuf *path,
- const char *normalized_objdir, khiter_t *pos)
-{
- int r;
-
- /* Detect cases where alternate disappeared */
- if (!is_directory(path->buf)) {
- error(_("object directory %s does not exist; "
- "check .git/objects/info/alternates"),
- path->buf);
- return 0;
- }
-
- /*
- * Prevent the common mistake of listing the same
- * thing twice, or object directory itself.
- */
- if (!o->odb_by_path) {
- khiter_t p;
-
- o->odb_by_path = kh_init_odb_path_map();
- assert(!o->odb->next);
- p = kh_put_odb_path_map(o->odb_by_path, o->odb->path, &r);
- if (r < 0) die_errno(_("kh_put_odb_path_map"));
- assert(r == 1); /* never used */
- kh_value(o->odb_by_path, p) = o->odb;
- }
- if (!fspathcmp(path->buf, normalized_objdir))
- return 0;
- *pos = kh_put_odb_path_map(o->odb_by_path, path->buf, &r);
- if (r < 0) die_errno(_("kh_put_odb_path_map"));
- /* r: 0 = exists, 1 = never used, 2 = deleted */
- return r == 0 ? 0 : 1;
-}
-
/*
* Prepare alternate object database registry.
*
@@ -575,6 +536,9 @@ static int link_alt_odb_entry(struct repository *r, const char *entry,
struct object_directory *ent;
struct strbuf pathbuf = STRBUF_INIT;
khiter_t pos;
+ int ret = -1;
+ int added;
+ struct raw_object_store *o = r->objects;
if (!is_absolute_path(entry) && relative_base) {
strbuf_realpath(&pathbuf, relative_base, 1);
@@ -585,8 +549,7 @@ static int link_alt_odb_entry(struct repository *r, const char *entry,
if (strbuf_normalize_path(&pathbuf) < 0 && relative_base) {
error(_("unable to normalize alternate object path: %s"),
pathbuf.buf);
- strbuf_release(&pathbuf);
- return -1;
+ goto out;
}
/*
@@ -596,11 +559,37 @@ static int link_alt_odb_entry(struct repository *r, const char *entry,
while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
strbuf_setlen(&pathbuf, pathbuf.len - 1);
- if (!alt_odb_usable(r->objects, &pathbuf, normalized_objdir, &pos)) {
- strbuf_release(&pathbuf);
- return -1;
+ /* Detect cases where alternate disappeared */
+ if (!is_directory(pathbuf.buf)) {
+ error(_("object directory %s does not exist; "
+ "check .git/objects/info/alternates"),
+ pathbuf.buf);
+ goto out;
+ }
+
+ /*
+ * Prevent the common mistake of listing the same
+ * thing twice, or object directory itself.
+ */
+ if (!o->odb_by_path) {
+ khiter_t p;
+
+ o->odb_by_path = kh_init_odb_path_map();
+ assert(!o->odb->next);
+ p = kh_put_odb_path_map(o->odb_by_path, o->odb->path, &added);
+ if (added < 0) die_errno(_("kh_put_odb_path_map"));
+ assert(added);
+ kh_value(o->odb_by_path, p) = o->odb;
}
+ if (!fspathcmp(pathbuf.buf, normalized_objdir))
+ goto out;
+
+ pos = kh_put_odb_path_map(o->odb_by_path, pathbuf.buf, &added);
+ if (added < 0) die_errno(_("kh_put_odb_path_map"));
+ if (!added)
+ goto out;
+
CALLOC_ARRAY(ent, 1);
/* pathbuf.buf is already in r->objects->odb_by_path */
ent->path = strbuf_detach(&pathbuf, NULL);
@@ -615,7 +604,10 @@ static int link_alt_odb_entry(struct repository *r, const char *entry,
/* recursively add alternates */
read_info_alternates(r, ent->path, depth + 1);
- return 0;
+ ret = 0;
+out:
+ strbuf_release(&pathbuf);
+ return ret;
}
static const char *parse_alt_odb_entry(const char *string,
