Hi brian On 03/04/2022 19:22, brian m. carlson wrote:
We allow several special forms of stash names in this code. In the future, we'll want to allow these same forms without parsing a stash commit, so let's refactor this code out into a function for reuse. Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> --- builtin/stash.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index 5897febfbe..4c281a5781 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -130,6 +130,24 @@ static void assert_stash_like(struct stash_info *info, const char *revision) die(_("'%s' is not a stash-like commit"), revision); }+static int parse_revision(struct strbuf *revision, const char *commit, int quiet)+{ + strbuf_init(revision, 0);
I think this should become strbuf_reset() and the caller should call strbuf_init() once before calling this function (or use STASH_INFO_INIT from ab/plug-leak-in-revisions). That should fix one of the leaks Ævar was talking about, otherwise we reallocate the strbuf each time this function is called and leak the previous allocation.
Best Wishes Phillip
+ if (!commit) { + if (!ref_exists(ref_stash)) { + fprintf_ln(stderr, _("No stash entries found.")); + return -1; + } + + strbuf_addf(revision, "%s@{0}", ref_stash); + } else if (strspn(commit, "0123456789") == strlen(commit)) { + strbuf_addf(revision, "%s@{%s}", ref_stash, commit); + } else { + strbuf_addstr(revision, commit); + } + return 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)) { - free_stash_info(info); - fprintf_ln(stderr, _("No stash entries found.")); - return -1; - } - - strbuf_addf(&info->revision, "%s@{0}", ref_stash); - } else if (strspn(commit, "0123456789") == strlen(commit)) { - strbuf_addf(&info->revision, "%s@{%s}", ref_stash, commit); - } else { - strbuf_addstr(&info->revision, commit); + if (parse_revision(&info->revision, commit, 0)) { + free_stash_info(info); + return -1; }revision = info->revision.buf;
