On Sun, Apr 03 2022, brian m. carlson wrote: > [...snip...] Per https://lore.kernel.org/git/220404.86tub9jql5.gmgdl@xxxxxxxxxxxxxxxxxxx/ there's a bug here, in the preceding commit you: +static int parse_revision(struct strbuf *revision, const char *commit, int quiet) +{ + strbuf_init(revision, 0); [...] static int get_stash_info(struct stash_info *info, int argc, const char **argv) { int ret; @@ -157,19 +175,9 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv) if (argc == 1) commit = argv[0]; - strbuf_init(&info->revision, 0); - if (!commit) { - if (!ref_exists(ref_stash)) { [...] + if (parse_revision(&info->revision, commit, 0)) { + free_stash_info(info); + return -1; } That caller wasn't dealing with an init'd "info", so we init'd it late and freed it (which my revisions_release() makes a lot less icky). But unrelated to that: > + if (argc) { > + /* > + * Find each specified stash, and load data into the array. > + */ > + for (i = 0; i < argc; i++) { > + ALLOC_GROW_BY(items, nitems, 1, nalloc); > + if (parse_revision(&revision, argv[i], 0) || Here... > + get_oid_with_context(the_repository, revision.buf, > + GET_OID_QUIETLY | GET_OID_GENTLY, > + &items[i], &unused)) { > + error(_("unable to find stash entry %s"), argv[i]); > + res = -1; > + goto out; > + } > + } > + } else { > + /* > + * Walk the reflog, finding each stash entry, and load data into the > + * array. > + */ > + for (i = 0;; i++) { > + char buf[32]; > + struct object_id oid; > + > + snprintf(buf, sizeof(buf), "%d", i); > + if (parse_revision(&revision, buf, 1) || ...and here you are leaking memory in a loop. Per the above you need a strbuf_reset() for both. Then (moved from earlier): > +static int write_commit_with_parents(struct object_id *out, const struct object_id *oid, struct commit_list *parents) > +{ > + size_t author_len, committer_len; > + struct commit *this; > + const char *orig_author, *orig_committer; > + char *author = NULL, *committer = NULL; > + const char *buffer; > + unsigned long bufsize; > + const char *p; > + struct strbuf msg = STRBUF_INIT; This is a leak, because: > +out: > + strbuf_reset(&msg); This should be strbuf_release(), not strbuf_reset(). Anyway, even if builtin/stash.c is currently a big fail whale it's still useful to test new code with SANITIZE=leak, e.g. (and most of this is a lot less painful on top of my in-flight topic) on top of yours you can try: printf "leak:%s\n" setup_revisions add_object_array_with_path \ add_rev_cmdline cmd_log_init_finish strvec_push \ get_untracked_files pump_io_round setup_rename_conflict_info parse_pathspec \ prep_parse_options init_reflog_walk >/tmp/supp.txt; LSAN_OPTIONS=print_suppressions=0:suppressions=/tmp/supp.txt ./t3903-stash.sh -vixd Where that printf is just something I came up with via trial & error by re-running the test with -vixd and quieting existing leaks, until getting to the first leak new in your topic.
