get_sha1_with_context: dynamically allocate oc->path

When a sha1 lookup returns the tree path via "struct
object_context", it just copies it into a fixed-size buffer.
This means the result can be truncated, and it means our
"struct object_context" consumes a lot of stack space.

Instead, let's allocate a string on the heap. Because most
callers don't care about this information, we'll avoid doing
it by default (so they don't all have to start calling
free() on the result). There are basically two options for
the caller to signal to us that it's interested:

  1. By setting a pointer to storage in the object_context.

  2. By passing a flag in another parameter.

Doing (1) would match the way that sha1_object_info_extended()
works. But it would mean that every caller would have to
initialize the object_context, which they don't currently
have to do.

This patch does (2), and adds a new bit to the function's
flags field. All of the callers that look at the "path"
field are updated to pass the new flag.

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

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 1890d7a6390dcae719af33ce5cb59e74832a80e8..421709517c765d83fa4a4abf00ff3b8a9dd79f64 100644 (file)
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -61,7 +61,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
        if (unknown_type)
                flags |= LOOKUP_UNKNOWN_OBJECT;
 
-       if (get_sha1_with_context(obj_name, 0, oid.hash, &obj_context))
+       if (get_sha1_with_context(obj_name, GET_SHA1_RECORD_PATH,
+                                 oid.hash, &obj_context))
                die("Not a valid object name %s", obj_name);
 
        if (!path)
@@ -165,6 +166,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
                die("git cat-file %s: bad file", obj_name);
 
        write_or_die(1, buf, size);
+       free(obj_context.path);
        return 0;
 }
 

diff --git a/builtin/grep.c b/builtin/grep.c
index 3ffb5b4e8176bbcd1ecf2e4ae2dc84aee968cd22..254c1c7849152884d9abd0b603f1c75f87e085b5 100644 (file)
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1190,7 +1190,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
                        break;
                }
 
-               if (get_sha1_with_context(arg, 0, oid.hash, &oc)) {
+               if (get_sha1_with_context(arg, GET_SHA1_RECORD_PATH,
+                                         oid.hash, &oc)) {
                        if (seen_dashdash)
                                die(_("unable to resolve revision: %s"), arg);
                        break;
@@ -1200,6 +1201,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
                if (!seen_dashdash)
                        verify_non_filename(prefix, arg);
                add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
+               free(oc.path);
        }
 
        /*

diff --git a/builtin/log.c b/builtin/log.c
index b3b10cc1edba2f6d77fdf4b12c328a7968fb5499..1db016bcd91c9c56ee2a3b146be64e40f2d1f7ed 100644 (file)
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -483,16 +483,20 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
            !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
                return stream_blob_to_fd(1, oid, NULL, 0);
 
-       if (get_sha1_with_context(obj_name, 0, oidc.hash, &obj_context))
+       if (get_sha1_with_context(obj_name, GET_SHA1_RECORD_PATH,
+                                 oidc.hash, &obj_context))
                die(_("Not a valid object name %s"), obj_name);
-       if (!obj_context.path[0] ||
-           !textconv_object(obj_context.path, obj_context.mode, &oidc, 1, &buf, &size))
+       if (!obj_context.path ||
+           !textconv_object(obj_context.path, obj_context.mode, &oidc, 1, &buf, &size)) {
+               free(obj_context.path);
                return stream_blob_to_fd(1, oid, NULL, 0);
+       }
 
        if (!buf)
                die(_("git show %s: bad file"), obj_name);
 
        write_or_die(1, buf, size);
+       free(obj_context.path);
        return 0;
 }
 

diff --git a/cache.h b/cache.h
index 6cae30f64c3c32790e5fed5a009f7c9fb5cf9b95..52b91f5b64146e1b6c7c93aa5054a7aca6303b42 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -1333,13 +1333,18 @@ static inline int hex2chr(const char *s)
 
 struct object_context {
        unsigned char tree[20];
-       char path[PATH_MAX];
        unsigned mode;
        /*
         * symlink_path is only used by get_tree_entry_follow_symlinks,
         * and only for symlinks that point outside the repository.
         */
        struct strbuf symlink_path;
+       /*
+        * If GET_SHA1_RECORD_PATH is set, this will record path (if any)
+        * found when resolving the name. The caller is responsible for
+        * releasing the memory.
+        */
+       char *path;
 };
 
 #define GET_SHA1_QUIETLY           01
@@ -1349,6 +1354,7 @@ struct object_context {
 #define GET_SHA1_TREEISH          020
 #define GET_SHA1_BLOB             040
 #define GET_SHA1_FOLLOW_SYMLINKS 0100
+#define GET_SHA1_RECORD_PATH     0200
 #define GET_SHA1_ONLY_TO_DIE    04000
 
 #define GET_SHA1_DISAMBIGUATORS \

diff --git a/sha1_name.c b/sha1_name.c
index e7b0393bfc845525662636581882837709a409df..5e2ec37b6557f59970ed2b2dd69d4e0b151d1fd0 100644 (file)
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1550,7 +1550,8 @@ static int get_sha1_with_context_1(const char *name,
                        namelen = strlen(cp);
                }
 
-               strlcpy(oc->path, cp, sizeof(oc->path));
+               if (flags & GET_SHA1_RECORD_PATH)
+                       oc->path = xstrdup(cp);
 
                if (!active_cache)
                        read_cache();
@@ -1613,7 +1614,8 @@ static int get_sha1_with_context_1(const char *name,
                                }
                        }
                        hashcpy(oc->tree, tree_sha1);
-                       strlcpy(oc->path, filename, sizeof(oc->path));
+                       if (flags & GET_SHA1_RECORD_PATH)
+                               oc->path = xstrdup(filename);
 
                        free(new_filename);
                        return ret;