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>

Jeff King committed May 19, 2017 at 08:54 UTC dc944b65f1d13e258edc88a7608e2fe957ec334e
5 files changed +24 -8
builtin/cat-file.c
+3 -1
@@ -61,7 +61,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
61 if (unknown_type)
62 flags |= LOOKUP_UNKNOWN_OBJECT;
63
64 - if (get_sha1_with_context(obj_name, 0, oid.hash, &obj_context))
64 + if (get_sha1_with_context(obj_name, GET_SHA1_RECORD_PATH,
65 + oid.hash, &obj_context))
66 die("Not a valid object name %s", obj_name);
67
68 if (!path)
@@ -165,6 +166,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
166 die("git cat-file %s: bad file", obj_name);
167
168 write_or_die(1, buf, size);
169 + free(obj_context.path);
170 return 0;
171 }
172
builtin/grep.c
+3 -1
@@ -1190,7 +1190,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
1190 break;
1191 }
1192
1193 - if (get_sha1_with_context(arg, 0, oid.hash, &oc)) {
1193 + if (get_sha1_with_context(arg, GET_SHA1_RECORD_PATH,
1194 + oid.hash, &oc)) {
1195 if (seen_dashdash)
1196 die(_("unable to resolve revision: %s"), arg);
1197 break;
@@ -1200,6 +1201,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
1201 if (!seen_dashdash)
1202 verify_non_filename(prefix, arg);
1203 add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
1204 + free(oc.path);
1205 }
1206
1207 /*
builtin/log.c
+7 -3
@@ -483,16 +483,20 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
483 !DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
484 return stream_blob_to_fd(1, oid, NULL, 0);
485
486 - if (get_sha1_with_context(obj_name, 0, oidc.hash, &obj_context))
486 + if (get_sha1_with_context(obj_name, GET_SHA1_RECORD_PATH,
487 + oidc.hash, &obj_context))
488 die(_("Not a valid object name %s"), obj_name);
488 - if (!obj_context.path[0] ||
489 - !textconv_object(obj_context.path, obj_context.mode, &oidc, 1, &buf, &size))
489 + if (!obj_context.path ||
490 + !textconv_object(obj_context.path, obj_context.mode, &oidc, 1, &buf, &size)) {
491 + free(obj_context.path);
492 return stream_blob_to_fd(1, oid, NULL, 0);
493 + }
494
495 if (!buf)
496 die(_("git show %s: bad file"), obj_name);
497
498 write_or_die(1, buf, size);
499 + free(obj_context.path);
500 return 0;
501 }
502
cache.h
+7 -1
@@ -1333,13 +1333,18 @@ static inline int hex2chr(const char *s)
1333
1334 struct object_context {
1335 unsigned char tree[20];
1336 - char path[PATH_MAX];
1336 unsigned mode;
1337 /*
1338 * symlink_path is only used by get_tree_entry_follow_symlinks,
1339 * and only for symlinks that point outside the repository.
1340 */
1341 struct strbuf symlink_path;
1342 + /*
1343 + * If GET_SHA1_RECORD_PATH is set, this will record path (if any)
1344 + * found when resolving the name. The caller is responsible for
1345 + * releasing the memory.
1346 + */
1347 + char *path;
1348 };
1349
1350 #define GET_SHA1_QUIETLY 01
@@ -1349,6 +1354,7 @@ struct object_context {
1354 #define GET_SHA1_TREEISH 020
1355 #define GET_SHA1_BLOB 040
1356 #define GET_SHA1_FOLLOW_SYMLINKS 0100
1357 +#define GET_SHA1_RECORD_PATH 0200
1358 #define GET_SHA1_ONLY_TO_DIE 04000
1359
1360 #define GET_SHA1_DISAMBIGUATORS \
sha1_name.c
+4 -2
@@ -1550,7 +1550,8 @@ static int get_sha1_with_context_1(const char *name,
1550 namelen = strlen(cp);
1551 }
1552
1553 - strlcpy(oc->path, cp, sizeof(oc->path));
1553 + if (flags & GET_SHA1_RECORD_PATH)
1554 + oc->path = xstrdup(cp);
1555
1556 if (!active_cache)
1557 read_cache();
@@ -1613,7 +1614,8 @@ static int get_sha1_with_context_1(const char *name,
1614 }
1615 }
1616 hashcpy(oc->tree, tree_sha1);
1616 - strlcpy(oc->path, filename, sizeof(oc->path));
1617 + if (flags & GET_SHA1_RECORD_PATH)
1618 + oc->path = xstrdup(filename);
1619
1620 free(new_filename);
1621 return ret;