| Message ID | xmqqsfyg82b5.fsf@gitster.g (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | a5221ea92e36051fcecdf479ec34279cfb0812cc |
| Headers | show |
| Series | hash-object: prefix_filename() returns allocated memory these days | expand |
On Tue, Sep 07, 2021 at 03:10:22PM -0700, Junio C Hamano wrote: > Back when a1be47e4 (hash-object: fix buffer reuse with --path in a > subdirectory, 2017-03-20) was written, the prefix_filename() helper > used a static piece of memory to the caller, making the caller > responsible for copying it, if it wants to keep it across another > call to the same function. Two callers of the prefix_filename() in > hash-object were made to xstrdup() the value obtained from it. > > But in the same series, when e4da43b1 (prefix_filename: return newly > allocated string, 2017-03-20) changed the rule to gave the caller > possession of the memory, we forgot to revert one of the xstrdup() > changes, allowing the returned value to leak. Good catch. The patch looks obviously correct to me. -Peff
diff --git c/builtin/hash-object.c w/builtin/hash-object.c index 640ef4ded5..084cba204b 100644 --- c/builtin/hash-object.c +++ w/builtin/hash-object.c @@ -117,7 +117,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix) prefix = setup_git_directory_gently(&nongit); if (vpath && prefix) - vpath = xstrdup(prefix_filename(prefix, vpath)); + vpath = prefix_filename(prefix, vpath); git_config(git_default_config, NULL);
Back when a1be47e4 (hash-object: fix buffer reuse with --path in a subdirectory, 2017-03-20) was written, the prefix_filename() helper used a static piece of memory to the caller, making the caller responsible for copying it, if it wants to keep it across another call to the same function. Two callers of the prefix_filename() in hash-object were made to xstrdup() the value obtained from it. But in the same series, when e4da43b1 (prefix_filename: return newly allocated string, 2017-03-20) changed the rule to gave the caller possession of the memory, we forgot to revert one of the xstrdup() changes, allowing the returned value to leak. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * "git show -W e4da43b1 builtin/hash-object.c" made me curious. builtin/hash-object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)