From c0a487eafb63301004af424bf02b1951abdc4da7 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 May 2017 08:52:06 -0400 Subject: sha1_name: consistently refer to object_context as "oc" An early version of the patch to add object_context used the name object_resolve_context. This was later shortened to just object_context, but the "orc" variable name stuck in a few places. Let's use "oc", which is used elsewhere in the code. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_name.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sha1_name.c') diff --git a/sha1_name.c b/sha1_name.c index 8eec9f7c1b..179f214c9e 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1638,9 +1638,9 @@ void maybe_die_on_misspelt_object_name(const char *name, const char *prefix) get_sha1_with_context_1(name, GET_SHA1_ONLY_TO_DIE, prefix, sha1, &oc); } -int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *orc) +int get_sha1_with_context(const char *str, unsigned flags, unsigned char *sha1, struct object_context *oc) { if (flags & GET_SHA1_FOLLOW_SYMLINKS && flags & GET_SHA1_ONLY_TO_DIE) die("BUG: incompatible flags for get_sha1_with_context"); - return get_sha1_with_context_1(str, flags, NULL, sha1, orc); + return get_sha1_with_context_1(str, flags, NULL, sha1, oc); } -- cgit v1.2.3 From d72cae12b9a7bba3a6626e0b5805955eafdefcc6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 May 2017 08:52:25 -0400 Subject: get_sha1_with_context: always initialize oc->symlink_path The get_sha1_with_context() function zeroes out the oc->symlink_path strbuf, but doesn't use strbuf_init() to set up the usual invariants (like pointing to the slopbuf). We don't actually write to the oc->symlink_path strbuf unless we call get_tree_entry_follow_symlinks(), and that function does initialize it. However, readers may still look at the zero'd strbuf. In practice this isn't a triggerable bug. The only caller that looks at it only does so when the mode we found is 0. This doesn't happen for non-tree-entries (where we return S_IFINVALID). A broken tree entry could have a mode of 0, but canon_mode() quietly rewrites that into S_IFGITLINK. So the "0" mode should only come up when we did indeed find a symlink. This is mostly just an accident of how the code happens to work, though. Let's future-proof ourselves to make sure the strbuf is properly initialized for all calls (it's only a few struct member assignments, not a heap allocation). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- sha1_name.c | 1 + 1 file changed, 1 insertion(+) (limited to 'sha1_name.c') diff --git a/sha1_name.c b/sha1_name.c index 179f214c9e..e7b0393bfc 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -1511,6 +1511,7 @@ static int get_sha1_with_context_1(const char *name, memset(oc, 0, sizeof(*oc)); oc->mode = S_IFINVALID; + strbuf_init(&oc->symlink_path, 0); ret = get_sha1_1(name, namelen, sha1, flags); if (!ret) return ret; -- cgit v1.2.3 From dc944b65f1d13e258edc88a7608e2fe957ec334e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 19 May 2017 08:54:43 -0400 Subject: 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 Signed-off-by: Junio C Hamano --- sha1_name.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'sha1_name.c') diff --git a/sha1_name.c b/sha1_name.c index e7b0393bfc..5e2ec37b65 100644 --- 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; -- cgit v1.2.3