{"thread":{"id":"53715","subject":"[PATCH] pull: plug minor memory leak after using is_descendant_of()","startedAt":"2020-06-19T13:14:35Z","lastAt":"2020-06-19T13:35:08Z","messageCount":2,"participants":["René Scharfe","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"400113","messageId":"52a584ee-ce2f-4416-3e3c-97ff15b5c346@web.de","threadId":"53715","inReplyTo":null,"subject":"[PATCH] pull: plug minor memory leak after using is_descendant_of()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2020-06-19T13:14:19Z","receivedAt":"2020-06-19T13:14:35Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"cmd_pull() builds a commit_list to pass a single potential ancestor to\nis_descendant_of().  The latter leaves the list intact.  Release the\nallocated memory after the call.\n\nLeaking in cmd_*() isn't a big deal, but sets a bad example for other\nusers of is_descendant_of().\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\nPatch generated with -U15 for easier review; only the pre-context is\ninteresting, though.  --function-context would add even more noise.\nA --block-context option might be nice (include surrounding lines up to\nand including the previous and next lines with lower indentation). :)\nOr perhaps it's a sign that the function should be split up..\n\n builtin/pull.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/builtin/pull.c b/builtin/pull.c\nindex 00e5857a8d..8e6572d305 100644\n--- a/builtin/pull.c\n+++ b/builtin/pull.c\n@@ -1019,30 +1019,31 @@ int cmd_pull(int argc, const char **argv, const char *prefix)\n \t\tif (!autostash) {\n \t\t\tstruct commit_list *list = NULL;\n \t\t\tstruct commit *merge_head, *head;\n\n \t\t\thead = lookup_commit_reference(the_repository,\n \t\t\t\t\t\t       &orig_head);\n \t\t\tcommit_list_insert(head, &list);\n \t\t\tmerge_head = lookup_commit_reference(the_repository,\n \t\t\t\t\t\t\t     &merge_heads.oid[0]);\n \t\t\tif (is_descendant_of(merge_head, list)) {\n \t\t\t\t/* we can fast-forward this without invoking rebase */\n \t\t\t\topt_ff = \"--ff-only\";\n \t\t\t\tran_ff = 1;\n \t\t\t\tret = run_merge();\n \t\t\t}\n+\t\t\tfree_commit_list(list);\n \t\t}\n \t\tif (!ran_ff)\n \t\t\tret = run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);\n\n \t\tif (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||\n \t\t\t     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))\n \t\t\tret = rebase_submodules();\n\n \t\treturn ret;\n \t} else {\n \t\tint ret = run_merge();\n \t\tif (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||\n \t\t\t     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))\n \t\t\tret = update_submodules();\n \t\treturn ret;\n--\n2.27.0\n"},{"id":"400120","messageId":"20200619133504.GC2540897@coredump.intra.peff.net","threadId":"53715","inReplyTo":"52a584ee-ce2f-4416-3e3c-97ff15b5c346@web.de","subject":"Re: [PATCH] pull: plug minor memory leak after using is_descendant_of()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-06-19T13:35:04Z","receivedAt":"2020-06-19T13:35:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jun 19, 2020 at 03:14:19PM +0200, René Scharfe wrote:\n\n> cmd_pull() builds a commit_list to pass a single potential ancestor to\n> is_descendant_of().  The latter leaves the list intact.  Release the\n> allocated memory after the call.\n> \n> Leaking in cmd_*() isn't a big deal, but sets a bad example for other\n> users of is_descendant_of().\n\nThis also looks good to me.\n\n> ---\n> Patch generated with -U15 for easier review; only the pre-context is\n> interesting, though.  --function-context would add even more noise.\n> A --block-context option might be nice (include surrounding lines up to\n> and including the previous and next lines with lower indentation). :)\n> Or perhaps it's a sign that the function should be split up..\n\nI wondered how:\n\n  git -c diff.cpp.funcname='.*{' show --function-context\n\nbut think it always goes to the \"^}\" line, which is more than we want\n(plus the start of the block is in the hunk header, which makes the\nindentation look funky).\n\n-Peff\n"}]}