Skip to content

Commit dc944b6

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d72cae1 commit dc944b6

File tree

5 files changed

+24
-8
lines changed

5 files changed

+24
-8
lines changed

builtin/cat-file.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
6161
if (unknown_type)
6262
flags |= LOOKUP_UNKNOWN_OBJECT;
6363

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))
6566
die("Not a valid object name %s", obj_name);
6667

6768
if (!path)
@@ -165,6 +166,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
165166
die("git cat-file %s: bad file", obj_name);
166167

167168
write_or_die(1, buf, size);
169+
free(obj_context.path);
168170
return 0;
169171
}
170172

builtin/grep.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1190,7 +1190,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
11901190
break;
11911191
}
11921192

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)) {
11941195
if (seen_dashdash)
11951196
die(_("unable to resolve revision: %s"), arg);
11961197
break;
@@ -1200,6 +1201,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
12001201
if (!seen_dashdash)
12011202
verify_non_filename(prefix, arg);
12021203
add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
1204+
free(oc.path);
12031205
}
12041206

12051207
/*

builtin/log.c

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,16 +483,20 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c
483483
!DIFF_OPT_TST(&rev->diffopt, ALLOW_TEXTCONV))
484484
return stream_blob_to_fd(1, oid, NULL, 0);
485485

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))
487488
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);
490492
return stream_blob_to_fd(1, oid, NULL, 0);
493+
}
491494

492495
if (!buf)
493496
die(_("git show %s: bad file"), obj_name);
494497

495498
write_or_die(1, buf, size);
499+
free(obj_context.path);
496500
return 0;
497501
}
498502

cache.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1333,13 +1333,18 @@ static inline int hex2chr(const char *s)
13331333

13341334
struct object_context {
13351335
unsigned char tree[20];
1336-
char path[PATH_MAX];
13371336
unsigned mode;
13381337
/*
13391338
* symlink_path is only used by get_tree_entry_follow_symlinks,
13401339
* and only for symlinks that point outside the repository.
13411340
*/
13421341
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;
13431348
};
13441349

13451350
#define GET_SHA1_QUIETLY 01
@@ -1349,6 +1354,7 @@ struct object_context {
13491354
#define GET_SHA1_TREEISH 020
13501355
#define GET_SHA1_BLOB 040
13511356
#define GET_SHA1_FOLLOW_SYMLINKS 0100
1357+
#define GET_SHA1_RECORD_PATH 0200
13521358
#define GET_SHA1_ONLY_TO_DIE 04000
13531359

13541360
#define GET_SHA1_DISAMBIGUATORS \

sha1_name.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1550,7 +1550,8 @@ static int get_sha1_with_context_1(const char *name,
15501550
namelen = strlen(cp);
15511551
}
15521552

1553-
strlcpy(oc->path, cp, sizeof(oc->path));
1553+
if (flags & GET_SHA1_RECORD_PATH)
1554+
oc->path = xstrdup(cp);
15541555

15551556
if (!active_cache)
15561557
read_cache();
@@ -1613,7 +1614,8 @@ static int get_sha1_with_context_1(const char *name,
16131614
}
16141615
}
16151616
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);
16171619

16181620
free(new_filename);
16191621
return ret;

0 commit comments

Comments
 (0)