link_alt_odb_entry: refactor string handling

The string handling in link_alt_odb_entry() is mostly an
artifact of the original version, which took the path as a
ptr/len combo, and did not have a NUL-terminated string
until we created one in the alternate_object_database
struct.  But since 5bdf0a8 (sha1_file: normalize alt_odb
path before comparing and storing, 2011-09-07), the first
thing we do is put the path into a strbuf, which gives us
some easy opportunities for cleanup.

In particular:

- we call strlen(pathbuf.buf), which is silly; we can look
at pathbuf.len.

- even though we have a strbuf, we don't maintain its
"len" field when chomping extra slashes from the
end, and instead keep a separate "pfxlen" variable. We
can fix this and then drop "pfxlen" entirely.

- we don't check whether the path is usable until after we
allocate the new struct, making extra cleanup work for
ourselves. Since we have a NUL-terminated string, we can
bump the "is it usable" checks higher in the function.
While we're at it, we can move that logic to its own
helper, which makes the flow of link_alt_odb_entry()
easier to follow.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_file.c b/sha1_file.c
index 057262d46ef31f4e15347442c853a164fce29889..3d8c1e88a53e23d8c6b93f1b8b915adf15b43ecd 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -234,6 +234,36 @@ char *sha1_pack_index_name(const unsigned char *sha1)
 struct alternate_object_database *alt_odb_list;
 static struct alternate_object_database **alt_odb_tail;
 
 struct alternate_object_database *alt_odb_list;
 static struct alternate_object_database **alt_odb_tail;
 

+/*
+ * Return non-zero iff the path is usable as an alternate object database.
+ */
+static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
+{
+       struct alternate_object_database *alt;
+
+       /* 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.
+        */
+       for (alt = alt_odb_list; alt; alt = alt->next) {
+               if (path->len == alt->name - alt->base - 1 &&
+                   !memcmp(path->buf, alt->base, path->len))
+                       return 0;
+       }
+       if (!fspathcmp(path->buf, normalized_objdir))
+               return 0;
+
+       return 1;
+}
+

 /*
  * Prepare alternate object database registry.
  *
 /*
  * Prepare alternate object database registry.
  *


@@ -253,8 +283,7 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
        int depth, const char *normalized_objdir)
 {
        struct alternate_object_database *ent;
        int depth, const char *normalized_objdir)
 {
        struct alternate_object_database *ent;

-       struct alternate_object_database *alt;
-       size_t pfxlen, entlen;
+       size_t entlen;

        struct strbuf pathbuf = STRBUF_INIT;
 
        if (!is_absolute_path(entry) && relative_base) {
        struct strbuf pathbuf = STRBUF_INIT;
 
        if (!is_absolute_path(entry) && relative_base) {


@@ -270,47 +299,26 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
                return -1;
        }
 
                return -1;
        }
 

-       pfxlen = strlen(pathbuf.buf);
-

        /*
         * The trailing slash after the directory name is given by
         * this function at the end. Remove duplicates.
         */
        /*
         * The trailing slash after the directory name is given by
         * this function at the end. Remove duplicates.
         */

-       while (pfxlen && pathbuf.buf[pfxlen-1] == '/')
-               pfxlen -= 1;
-
-       entlen = st_add(pfxlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
-       ent = xmalloc(st_add(sizeof(*ent), entlen));
-       memcpy(ent->base, pathbuf.buf, pfxlen);
-       strbuf_release(&pathbuf);
-
-       ent->name = ent->base + pfxlen + 1;
-       ent->base[pfxlen + 3] = '/';
-       ent->base[pfxlen] = ent->base[entlen-1] = 0;
+       while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
+               strbuf_setlen(&pathbuf, pathbuf.len - 1);

 
 

-       /* Detect cases where alternate disappeared */
-       if (!is_directory(ent->base)) {
-               error("object directory %s does not exist; "
-                     "check .git/objects/info/alternates.",
-                     ent->base);
-               free(ent);
+       if (!alt_odb_usable(&pathbuf, normalized_objdir)) {
+               strbuf_release(&pathbuf);

                return -1;
        }
 
                return -1;
        }
 

-       /* Prevent the common mistake of listing the same
-        * thing twice, or object directory itself.
-        */
-       for (alt = alt_odb_list; alt; alt = alt->next) {
-               if (pfxlen == alt->name - alt->base - 1 &&
-                   !memcmp(ent->base, alt->base, pfxlen)) {
-                       free(ent);
-                       return -1;
-               }
-       }
-       if (!fspathcmp(ent->base, normalized_objdir)) {
-               free(ent);
-               return -1;
-       }
+       entlen = st_add(pathbuf.len, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
+       ent = xmalloc(st_add(sizeof(*ent), entlen));
+       memcpy(ent->base, pathbuf.buf, pathbuf.len);
+
+       ent->name = ent->base + pathbuf.len + 1;
+       ent->base[pathbuf.len] = '/';
+       ent->base[pathbuf.len + 3] = '/';
+       ent->base[entlen-1] = 0;

 
        /* add the alternate entry */
        *alt_odb_tail = ent;
 
        /* add the alternate entry */
        *alt_odb_tail = ent;


@@ -318,10 +326,9 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
        ent->next = NULL;
 
        /* recursively add alternates */
        ent->next = NULL;
 
        /* recursively add alternates */

-       read_info_alternates(ent->base, depth + 1);
-
-       ent->base[pfxlen] = '/';
+       read_info_alternates(pathbuf.buf, depth + 1);

 
 

+       strbuf_release(&pathbuf);

        return 0;
 }
 
        return 0;
 }