| Message ID | patch-v3-13.19-6d99fdcc44e-20230110T054138Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | leak fixes: various simple leak fixes | expand |
Am 10.01.23 um 06:43 schrieb Ævar Arnfjörð Bjarmason: > Follow-up 465028e0e25 (merge: add missing strbuf_release(), > 2021-10-07) and address the "msg" memory leak in this block. We could > free "&msg" before the "goto done" here, but even better is to avoid > allocating it in the first place. So contrary to the subject we don't need to free it anymore. > > By repeating the "Fast-forward" string here we can avoid using a > "struct strbuf" altogether. > > Suggested-by: René Scharfe <l.s.r@web.de> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > builtin/merge.c | 11 ++++------- > t/t6439-merge-co-error-msgs.sh | 1 + > 2 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/builtin/merge.c b/builtin/merge.c > index 0f093f2a4f2..91dd5435c59 100644 > --- a/builtin/merge.c > +++ b/builtin/merge.c > @@ -1560,7 +1560,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) > !common->next && > oideq(&common->item->object.oid, &head_commit->object.oid)) { > /* Again the most common case of merging one remote. */ > - struct strbuf msg = STRBUF_INIT; > + const char *msg = have_message ? > + "Fast-forward (no commit created; -m option ignored)" : > + "Fast-forward"; > struct commit *commit; > > if (verbosity >= 0) { > @@ -1570,10 +1572,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix) > find_unique_abbrev(&remoteheads->item->object.oid, > DEFAULT_ABBREV)); > } > - strbuf_addstr(&msg, "Fast-forward"); > - if (have_message) > - strbuf_addstr(&msg, > - " (no commit created; -m option ignored)"); > commit = remoteheads->item; > if (!commit) { > ret = 1; > @@ -1592,9 +1590,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix) > goto done; > } > > - finish(head_commit, remoteheads, &commit->object.oid, msg.buf); > + finish(head_commit, remoteheads, &commit->object.oid, msg); > remove_merge_branch_state(the_repository); > - strbuf_release(&msg); > goto done; > } else if (!remoteheads->next && common->next) > ; > diff --git a/t/t6439-merge-co-error-msgs.sh b/t/t6439-merge-co-error-msgs.sh > index 52cf0c87690..0cbec57cdab 100755 > --- a/t/t6439-merge-co-error-msgs.sh > +++ b/t/t6439-merge-co-error-msgs.sh > @@ -5,6 +5,7 @@ test_description='unpack-trees error messages' > GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main > export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME > > +TEST_PASSES_SANITIZE_LEAK=true > . ./test-lib.sh > >
René Scharfe <l.s.r@web.de> writes: > Am 10.01.23 um 06:43 schrieb Ævar Arnfjörð Bjarmason: >> Follow-up 465028e0e25 (merge: add missing strbuf_release(), >> 2021-10-07) and address the "msg" memory leak in this block. We could >> free "&msg" before the "goto done" here, but even better is to avoid >> allocating it in the first place. > > So contrary to the subject we don't need to free it anymore. ;-) I appreciate such careful reading.
diff --git a/builtin/merge.c b/builtin/merge.c index 0f093f2a4f2..91dd5435c59 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1560,7 +1560,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix) !common->next && oideq(&common->item->object.oid, &head_commit->object.oid)) { /* Again the most common case of merging one remote. */ - struct strbuf msg = STRBUF_INIT; + const char *msg = have_message ? + "Fast-forward (no commit created; -m option ignored)" : + "Fast-forward"; struct commit *commit; if (verbosity >= 0) { @@ -1570,10 +1572,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix) find_unique_abbrev(&remoteheads->item->object.oid, DEFAULT_ABBREV)); } - strbuf_addstr(&msg, "Fast-forward"); - if (have_message) - strbuf_addstr(&msg, - " (no commit created; -m option ignored)"); commit = remoteheads->item; if (!commit) { ret = 1; @@ -1592,9 +1590,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix) goto done; } - finish(head_commit, remoteheads, &commit->object.oid, msg.buf); + finish(head_commit, remoteheads, &commit->object.oid, msg); remove_merge_branch_state(the_repository); - strbuf_release(&msg); goto done; } else if (!remoteheads->next && common->next) ; diff --git a/t/t6439-merge-co-error-msgs.sh b/t/t6439-merge-co-error-msgs.sh index 52cf0c87690..0cbec57cdab 100755 --- a/t/t6439-merge-co-error-msgs.sh +++ b/t/t6439-merge-co-error-msgs.sh @@ -5,6 +5,7 @@ test_description='unpack-trees error messages' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh
Follow-up 465028e0e25 (merge: add missing strbuf_release(), 2021-10-07) and address the "msg" memory leak in this block. We could free "&msg" before the "goto done" here, but even better is to avoid allocating it in the first place. By repeating the "Fast-forward" string here we can avoid using a "struct strbuf" altogether. Suggested-by: René Scharfe <l.s.r@web.de> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/merge.c | 11 ++++------- t/t6439-merge-co-error-msgs.sh | 1 + 2 files changed, 5 insertions(+), 7 deletions(-)