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>

diff --git a/cache.h b/cache.h
index 555ba713de6efd9c1d9e5b9bc35fb29521b9dc43..5d36ffa1f253d1c007bfa41be270593612d44a6e 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1384,8 +1384,9 @@ extern void remove_scheduled_dirs(void);
 extern struct alternate_object_database {
        struct alternate_object_database *next;
 
 extern struct alternate_object_database {
        struct alternate_object_database *next;
 

-       char *name;
-       char *scratch;
+       /* see alt_scratch_buf() */
+       struct strbuf scratch;
+       size_t base_len;

 
        char path[FLEX_ARRAY];
 } *alt_odb_list;
 
        char path[FLEX_ARRAY];
 } *alt_odb_list;


@@ -1414,6 +1415,14 @@ extern void add_to_alternates_file(const char *dir);
  */
 extern void add_to_alternates_memory(const char *dir);
 
  */
 extern void add_to_alternates_memory(const char *dir);
 

+/*
+ * Returns a scratch strbuf pre-filled with the alternate object directory,
+ * including a trailing slash, which can be used to access paths in the
+ * alternate. Always use this over direct access to alt->scratch, as it
+ * cleans up any previous use of the scratch buffer.
+ */
+extern struct strbuf *alt_scratch_buf(struct alternate_object_database *alt);
+

 struct pack_window {
        struct pack_window *next;
        unsigned char *base;
 struct pack_window {
        struct pack_window *next;
        unsigned char *base;

diff --git a/sha1_file.c b/sha1_file.c
index da7b9226051bf27a3a5802544b6527b3d6121824..51d40241a742839d0740a28144ef111c00cb1091 100644 (file)
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -204,11 +204,24 @@ const char *sha1_file_name(const unsigned char *sha1)
        return buf;
 }
 
        return buf;
 }
 

+struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
+{
+       strbuf_setlen(&alt->scratch, alt->base_len);
+       return &alt->scratch;
+}
+

 static const char *alt_sha1_path(struct alternate_object_database *alt,
                                 const unsigned char *sha1)
 {
 static const char *alt_sha1_path(struct alternate_object_database *alt,
                                 const unsigned char *sha1)
 {

-       fill_sha1_path(alt->name, sha1);
-       return alt->scratch;
+       /* hex sha1 plus internal "/" */
+       size_t len = GIT_SHA1_HEXSZ + 1;
+       struct strbuf *buf = alt_scratch_buf(alt);
+
+       strbuf_grow(buf, len);
+       fill_sha1_path(buf->buf + buf->len, sha1);
+       strbuf_setlen(buf, buf->len + len);
+
+       return buf->buf;

 }
 
 /*
 }
 
 /*


@@ -396,16 +409,11 @@ void read_info_alternates(const char * relative_base, int depth)
 struct alternate_object_database *alloc_alt_odb(const char *dir)
 {
        struct alternate_object_database *ent;
 struct alternate_object_database *alloc_alt_odb(const char *dir)
 {
        struct alternate_object_database *ent;

-       size_t dirlen = strlen(dir);
-       size_t entlen;

 
 

-       entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */

        FLEX_ALLOC_STR(ent, path, dir);
        FLEX_ALLOC_STR(ent, path, dir);

-       ent->scratch = xmalloc(entlen);
-       xsnprintf(ent->scratch, entlen, "%s/", dir);
-
-       ent->name = ent->scratch + dirlen + 1;
-       ent->scratch[dirlen] = '/';
+       strbuf_init(&ent->scratch, 0);
+       strbuf_addf(&ent->scratch, "%s/", dir);
+       ent->base_len = ent->scratch.len;

 
        return ent;
 }
 
        return ent;
 }

diff --git a/sha1_name.c b/sha1_name.c
index 770ea4fe805969b68fdbc65fed769cc5202440ff..defbb3eb05aa3d82c86549337f9cc56e1f6076bd 100644 (file)
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -92,15 +92,12 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
 
        xsnprintf(hex, sizeof(hex), "%.2s", hex_pfx);
        for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
 
        xsnprintf(hex, sizeof(hex), "%.2s", hex_pfx);
        for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {

+               struct strbuf *buf = alt_scratch_buf(alt);

                struct dirent *de;
                DIR *dir;
 
                struct dirent *de;
                DIR *dir;
 

-               /*
-                * every alt_odb struct has 42 extra bytes after the base
-                * for exactly this purpose
-                */
-               xsnprintf(alt->name, 42, "%.2s/", hex_pfx);
-               dir = opendir(alt->scratch);
+               strbuf_addf(buf, "%.2s/", hex_pfx);
+               dir = opendir(buf->buf);

                if (!dir)
                        continue;
 
                if (!dir)
                        continue;