@matheusbsbs / matheus-bagliano / commits / f87c55c264

object-name: free leaking object contexts

While it is documented in `struct object_context::path` that this variable needs to be released by the caller, this fact is rather easy to miss given that we do not ever provide a function to release the object context. And of course, while some callers dutifully release the path, many others don't. Introduce a new `object_context_release()` function that releases the path. Convert callsites that used to free the path to use that new function and add missing calls to callsites that were leaking memory. Refactor those callsites as required to have a single return path, only. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jun 11, 2024 at 11:19 UTC f87c55c2647cf3aa0e6b5e45738facb6b62fe37c

11 files changed +97 -47

builtin/cat-file.c

+11 -6

@@ -102,7 +102,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,

102 enum object_type type;

103 char *buf;

104 unsigned long size;

105 - struct object_context obj_context;

105 + struct object_context obj_context = {0};

106 struct object_info oi = OBJECT_INFO_INIT;

107 struct strbuf sb = STRBUF_INIT;

108 unsigned flags = OBJECT_INFO_LOOKUP_REPLACE;

@@ -163,7 +163,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,

163 goto cleanup;

164

165 case 'e':

166 - return !repo_has_object_file(the_repository, &oid);

166 + ret = !repo_has_object_file(the_repository, &oid);

167 + goto cleanup;

168

169 case 'w':

170

@@ -268,7 +269,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,

269 ret = 0;

270 cleanup:

271 free(buf);

271 - free(obj_context.path);

272 + object_context_release(&obj_context);

273 return ret;

274 }

275

@@ -520,7 +521,7 @@ static void batch_one_object(const char *obj_name,

521 struct batch_options *opt,

522 struct expand_data *data)

523 {

523 - struct object_context ctx;

524 + struct object_context ctx = {0};

525 int flags =

526 GET_OID_HASH_ANY |

527 (opt->follow_symlinks ? GET_OID_FOLLOW_SYMLINKS : 0);

@@ -557,7 +558,8 @@ static void batch_one_object(const char *obj_name,

558 break;

559 }

560 fflush(stdout);

560 - return;

561 +

562 + goto out;

563 }

564

565 if (ctx.mode == 0) {

@@ -565,10 +567,13 @@ static void batch_one_object(const char *obj_name,

567 (uintmax_t)ctx.symlink_path.len,

568 opt->output_delim, ctx.symlink_path.buf, opt->output_delim);

569 fflush(stdout);

568 - return;

570 + goto out;

571 }

572

573 batch_object_write(obj_name, scratch, opt, data, NULL, 0);

574 +

575 +out:

576 + object_context_release(&ctx);

577 }

578

579 struct object_cb_data {

builtin/grep.c

+2 -2

@@ -1114,7 +1114,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)

1114 for (i = 0; i < argc; i++) {

1115 const char *arg = argv[i];

1116 struct object_id oid;

1117 - struct object_context oc;

1117 + struct object_context oc = {0};

1118 struct object *object;

1119

1120 if (!strcmp(arg, "--")) {

@@ -1140,7 +1140,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)

1140 if (!seen_dashdash)

1141 verify_non_filename(prefix, arg);

1142 add_object_array_with_path(object, arg, &list, oc.mode, oc.path);

1143 - free(oc.path);

1143 + object_context_release(&oc);

1144 }

1145

1146 /*

builtin/log.c

+3 -3

@@ -682,7 +682,7 @@ static void show_tagger(const char *buf, struct rev_info *rev)

682 static int show_blob_object(const struct object_id *oid, struct rev_info *rev, const char *obj_name)

683 {

684 struct object_id oidc;

685 - struct object_context obj_context;

685 + struct object_context obj_context = {0};

686 char *buf;

687 unsigned long size;

688

@@ -698,7 +698,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c

698 if (!obj_context.path ||

699 !textconv_object(the_repository, obj_context.path,

700 obj_context.mode, &oidc, 1, &buf, &size)) {

701 - free(obj_context.path);

701 + object_context_release(&obj_context);

702 return stream_blob_to_fd(1, oid, NULL, 0);

703 }

704

@@ -706,7 +706,7 @@ static int show_blob_object(const struct object_id *oid, struct rev_info *rev, c

706 die(_("git show %s: bad file"), obj_name);

707

708 write_or_die(1, buf, size);

709 - free(obj_context.path);

709 + object_context_release(&obj_context);

710 return 0;

711 }

712

builtin/ls-tree.c

+2 -1

@@ -367,7 +367,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)

367 OPT_END()

368 };

369 struct ls_tree_cmdmode_to_fmt *m2f = ls_tree_cmdmode_format;

370 - struct object_context obj_context;

370 + struct object_context obj_context = {0};

371 int ret;

372

373 git_config(git_default_config, NULL);

@@ -441,5 +441,6 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)

441

442 ret = !!read_tree(the_repository, tree, &options.pathspec, fn, &options);

443 clear_pathspec(&options.pathspec);

444 + object_context_release(&obj_context);

445 return ret;

446 }

builtin/rev-parse.c

@@ -1128,6 +1128,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)

1128 }

1129 if (!get_oid_with_context(the_repository, name,

1130 flags, &oid, &unused)) {

1131 + object_context_release(&unused);

1132 if (output_algo)

1133 repo_oid_to_algop(the_repository, &oid,

1134 output_algo, &oid);

@@ -1137,6 +1138,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)

1138 show_rev(type, &oid, name);

1139 continue;

1140 }

1141 + object_context_release(&unused);

1142 if (verify)

1143 die_no_single_rev(quiet);

1144 if (has_dashdash)

builtin/stash.c

+9 -3

@@ -1018,13 +1018,14 @@ static int store_stash(int argc, const char **argv, const char *prefix)

1018 int quiet = 0;

1019 const char *stash_msg = NULL;

1020 struct object_id obj;

1021 - struct object_context dummy;

1021 + struct object_context dummy = {0};

1022 struct option options[] = {

1023 OPT__QUIET(&quiet, N_("be quiet")),

1024 OPT_STRING('m', "message", &stash_msg, "message",

1025 N_("stash message")),

1026 OPT_END()

1027 };

1028 + int ret;

1029

1030 argc = parse_options(argc, argv, prefix, options,

1031 git_stash_store_usage,

@@ -1043,10 +1044,15 @@ static int store_stash(int argc, const char **argv, const char *prefix)

1044 if (!quiet)

1045 fprintf_ln(stderr, _("Cannot update %s with %s"),

1046 ref_stash, argv[0]);

1046 - return -1;

1047 + ret = -1;

1048 + goto out;

1049 }

1050

1049 - return do_store_stash(&obj, stash_msg, quiet);

1051 + ret = do_store_stash(&obj, stash_msg, quiet);

1052 +

1053 +out:

1054 + object_context_release(&dummy);

1055 + return ret;

1056 }

1057

1058 static void add_pathspecs(struct strvec *args,

list-objects-filter.c

@@ -542,6 +542,8 @@ static void filter_sparse_oid__init(

542 filter->filter_data = d;

543 filter->filter_object_fn = filter_sparse;

544 filter->free_fn = filter_sparse_free;

545 +

546 + object_context_release(&oc);

547 }

548

549 /*

object-name.c

+29 -11

@@ -1757,6 +1757,11 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)

1757 return check_refname_format(sb->buf, 0);

1758 }

1759

1760 +void object_context_release(struct object_context *ctx)

1761 +{

1762 + free(ctx->path);

1763 +}

1764 +

1765 /*

1766 * This is like "get_oid_basic()", except it allows "object ID expressions",

1767 * notably "xyz^" for "parent of xyz"

@@ -1764,7 +1769,9 @@ int strbuf_check_branch_ref(struct strbuf *sb, const char *name)

1769 int repo_get_oid(struct repository *r, const char *name, struct object_id *oid)

1770 {

1771 struct object_context unused;

1767 - return get_oid_with_context(r, name, 0, oid, &unused);

1772 + int ret = get_oid_with_context(r, name, 0, oid, &unused);

1773 + object_context_release(&unused);

1774 + return ret;

1775 }

1776

1777 /*

@@ -1802,8 +1809,10 @@ int repo_get_oid_committish(struct repository *r,

1809 struct object_id *oid)

1810 {

1811 struct object_context unused;

1805 - return get_oid_with_context(r, name, GET_OID_COMMITTISH,

1806 - oid, &unused);

1812 + int ret = get_oid_with_context(r, name, GET_OID_COMMITTISH,

1813 + oid, &unused);

1814 + object_context_release(&unused);

1815 + return ret;

1816 }

1817

1818 int repo_get_oid_treeish(struct repository *r,

@@ -1811,8 +1820,10 @@ int repo_get_oid_treeish(struct repository *r,

1820 struct object_id *oid)

1821 {

1822 struct object_context unused;

1814 - return get_oid_with_context(r, name, GET_OID_TREEISH,

1815 - oid, &unused);

1823 + int ret = get_oid_with_context(r, name, GET_OID_TREEISH,

1824 + oid, &unused);

1825 + object_context_release(&unused);

1826 + return ret;

1827 }

1828

1829 int repo_get_oid_commit(struct repository *r,

@@ -1820,8 +1831,10 @@ int repo_get_oid_commit(struct repository *r,

1831 struct object_id *oid)

1832 {

1833 struct object_context unused;

1823 - return get_oid_with_context(r, name, GET_OID_COMMIT,

1824 - oid, &unused);

1834 + int ret = get_oid_with_context(r, name, GET_OID_COMMIT,

1835 + oid, &unused);

1836 + object_context_release(&unused);

1837 + return ret;

1838 }

1839

1840 int repo_get_oid_tree(struct repository *r,

@@ -1829,8 +1842,10 @@ int repo_get_oid_tree(struct repository *r,

1842 struct object_id *oid)

1843 {

1844 struct object_context unused;

1832 - return get_oid_with_context(r, name, GET_OID_TREE,

1833 - oid, &unused);

1845 + int ret = get_oid_with_context(r, name, GET_OID_TREE,

1846 + oid, &unused);

1847 + object_context_release(&unused);

1848 + return ret;

1849 }

1850

1851 int repo_get_oid_blob(struct repository *r,

@@ -1838,8 +1853,10 @@ int repo_get_oid_blob(struct repository *r,

1853 struct object_id *oid)

1854 {

1855 struct object_context unused;

1841 - return get_oid_with_context(r, name, GET_OID_BLOB,

1842 - oid, &unused);

1856 + int ret = get_oid_with_context(r, name, GET_OID_BLOB,

1857 + oid, &unused);

1858 + object_context_release(&unused);

1859 + return ret;

1860 }

1861

1862 /* Must be called only when object_name:filename doesn't exist. */

@@ -2117,6 +2134,7 @@ void maybe_die_on_misspelt_object_name(struct repository *r,

2134 struct object_id oid;

2135 get_oid_with_context_1(r, name, GET_OID_ONLY_TO_DIE | GET_OID_QUIETLY,

2136 prefix, &oid, &oc);

2137 + object_context_release(&oc);

2138 }

2139

2140 enum get_oid_result get_oid_with_context(struct repository *repo,

object-name.h

@@ -22,6 +22,8 @@ struct object_context {

22 char *path;

23 };

25 +void object_context_release(struct object_context *ctx);

26 +

27 /*

28 * Return an abbreviated sha1 unique within this repository's object database.

29 * The result will be at least `len` characters long, and will be NUL

revision.c

+34 -21

@@ -2130,30 +2130,26 @@ static int handle_dotdot(const char *arg,

2130 struct rev_info *revs, int flags,

2131 int cant_be_filename)

2132 {

2133 - struct object_context a_oc, b_oc;

2133 + struct object_context a_oc = {0}, b_oc = {0};

2134 char *dotdot = strstr(arg, "..");

2135 int ret;

2136

2137 if (!dotdot)

2138 return -1;

2139

2140 - memset(&a_oc, 0, sizeof(a_oc));

2141 - memset(&b_oc, 0, sizeof(b_oc));

2142 -

2140 *dotdot = '\0';

2141 ret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename,

2142 &a_oc, &b_oc);

2143 *dotdot = '.';

2144

2148 - free(a_oc.path);

2149 - free(b_oc.path);

2150 -

2145 + object_context_release(&a_oc);

2146 + object_context_release(&b_oc);

2147 return ret;

2148 }

2149

2150 static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int flags, unsigned revarg_opt)

2151 {

2156 - struct object_context oc;

2152 + struct object_context oc = {0};

2153 char *mark;

2154 struct object *object;

2155 struct object_id oid;

@@ -2161,6 +2157,7 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl

2157 const char *arg = arg_;

2158 int cant_be_filename = revarg_opt & REVARG_CANNOT_BE_FILENAME;

2159 unsigned get_sha1_flags = GET_OID_RECORD_PATH;

2160 + int ret;

2161

2162 flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;

2163

@@ -2169,17 +2166,22 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl

2166 * Just ".."? That is not a range but the

2167 * pathspec for the parent directory.

2168 */

2172 - return -1;

2169 + ret = -1;

2170 + goto out;

2171 }

2172

2175 - if (!handle_dotdot(arg, revs, flags, revarg_opt))

2176 - return 0;

2173 + if (!handle_dotdot(arg, revs, flags, revarg_opt)) {

2174 + ret = 0;

2175 + goto out;

2176 + }

2177

2178 mark = strstr(arg, "^@");

2179 if (mark && !mark[2]) {

2180 *mark = 0;

2181 - if (add_parents_only(revs, arg, flags, 0))

2182 - return 0;

2181 + if (add_parents_only(revs, arg, flags, 0)) {

2182 + ret = 0;

2183 + goto out;

2184 + }

2185 *mark = '^';

2186 }

2187 mark = strstr(arg, "^!");

@@ -2194,8 +2196,10 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl

2196

2197 if (mark[2]) {

2198 if (strtol_i(mark + 2, 10, &exclude_parent) ||

2197 - exclude_parent < 1)

2198 - return -1;

2199 + exclude_parent < 1) {

2200 + ret = -1;

2201 + goto out;

2202 + }

2203 }

2204

2205 *mark = 0;

@@ -2217,17 +2221,25 @@ static int handle_revision_arg_1(const char *arg_, struct rev_info *revs, int fl

2221 * should error out if we can't even get an oid, as

2222 * `--missing=print` should be able to report missing oids.

2223 */

2220 - if (get_oid_with_context(revs->repo, arg, get_sha1_flags, &oid, &oc))

2221 - return revs->ignore_missing ? 0 : -1;

2224 + if (get_oid_with_context(revs->repo, arg, get_sha1_flags, &oid, &oc)) {

2225 + ret = revs->ignore_missing ? 0 : -1;

2226 + goto out;

2227 + }

2228 if (!cant_be_filename)

2229 verify_non_filename(revs->prefix, arg);

2230 object = get_reference(revs, arg, &oid, flags ^ local_flags);

2225 - if (!object)

2226 - return (revs->ignore_missing || revs->do_not_die_on_missing_objects) ? 0 : -1;

2231 + if (!object) {

2232 + ret = (revs->ignore_missing || revs->do_not_die_on_missing_objects) ? 0 : -1;

2233 + goto out;

2234 + }

2235 add_rev_cmdline(revs, object, arg_, REV_CMD_REV, flags ^ local_flags);

2236 add_pending_object_with_path(revs, object, arg, oc.mode, oc.path);

2229 - free(oc.path);

2230 - return 0;

2237 +

2238 + ret = 0;

2239 +

2240 +out:

2241 + object_context_release(&oc);

2242 + return ret;

2243 }

2244

2245 int handle_revision_arg(const char *arg, struct rev_info *revs, int flags, unsigned revarg_opt)

@@ -3062,6 +3074,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s

3074 diagnose_missing_default(revs->def);

3075 object = get_reference(revs, revs->def, &oid, 0);

3076 add_pending_object_with_mode(revs, object, revs->def, oc.mode);

3077 + object_context_release(&oc);

3078 }

3079

3080 /* Did the user ask for any diff output? Run the diff! */

t/t7012-skip-worktree-writing.sh

@@ -5,6 +5,7 @@

6 test_description='test worktree writing operations when skip-worktree is used'

8 +TEST_PASSES_SANITIZE_LEAK=true

9 . ./test-lib.sh

11 test_expect_success 'setup' '