* [Outreachy] [PATCH] bisect--helper: refer branch.buf before strbuf_release(...) @ 2019-12-08 17:28 Miriam Rubio 2019-12-08 19:11 ` Johannes Schindelin 0 siblings, 1 reply; 3+ messages in thread From: Miriam Rubio @ 2019-12-08 17:28 UTC (permalink / raw) To: git; +Cc: Tanushree Tumane, Johannes Schindelin, Christian Couder, Miriam Rubio From: Tanushree Tumane <tanushreetumane@gmail.com> Move `error("...%s...", branch.buf);` before `strbuf_release(&branch);` to release string buffer `branch` and the memory it used. Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Mentored-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com> Signed-off-by: Miriam Rubio <mirucam@gmail.com> --- builtin/bisect--helper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c index 1fbe156e67..3055b2bb50 100644 --- a/builtin/bisect--helper.c +++ b/builtin/bisect--helper.c @@ -169,11 +169,12 @@ static int bisect_reset(const char *commit) argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL); if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) { + error(_("could not check out original" + " HEAD '%s'. Try 'git bisect" + " reset <commit>'."), branch.buf); strbuf_release(&branch); argv_array_clear(&argv); - return error(_("could not check out original" - " HEAD '%s'. Try 'git bisect" - " reset <commit>'."), branch.buf); + return -1; } argv_array_clear(&argv); } -- 2.21.0 (Apple Git-122.2) ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Outreachy] [PATCH] bisect--helper: refer branch.buf before strbuf_release(...) 2019-12-08 17:28 [Outreachy] [PATCH] bisect--helper: refer branch.buf before strbuf_release(...) Miriam Rubio @ 2019-12-08 19:11 ` Johannes Schindelin 2019-12-09 8:15 ` Miriam R. 0 siblings, 1 reply; 3+ messages in thread From: Johannes Schindelin @ 2019-12-08 19:11 UTC (permalink / raw) To: Miriam Rubio; +Cc: git, Tanushree Tumane, Christian Couder Hi Miriam, welcome to the Git project! On Sun, 8 Dec 2019, Miriam Rubio wrote: > From: Tanushree Tumane <tanushreetumane@gmail.com> Is this really a patch authored by Tanushree, or is this a fix written by you, intended to fix a patch authored by Tanushree? If it is the latter, please use a completely new commit message and take the authorship yourself. > Move `error("...%s...", branch.buf);` before `strbuf_release(&branch);` > to release string buffer `branch` and the memory it used. This describes the "what?", but the patch already does that. The commit message should be more about the "why?". In this instance, I believe that the commit message should read more like this: -- snip -- bisect--helper: avoid free-after-use In 5e82c3dd22a (bisect--helper: `bisect_reset` shell function in C, 2019-01-02), the `git bisect reset` subcommand was ported to C. When the call to `git checkout` failed, an error message was reported to the user. However, this error message used the `strbuf` that had just been released already. Let's switch that around: first use it, then release it. Signed-off-by: Miriam Rubio <mirucam@gmail.com> -- snap -- > > Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> > Mentored-by: Christian Couder <chriscool@tuxfamily.org> > Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com> > Signed-off-by: Miriam Rubio <mirucam@gmail.com> > --- > builtin/bisect--helper.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > index 1fbe156e67..3055b2bb50 100644 > --- a/builtin/bisect--helper.c > +++ b/builtin/bisect--helper.c > @@ -169,11 +169,12 @@ static int bisect_reset(const char *commit) > > argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL); > if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) { > + error(_("could not check out original" > + " HEAD '%s'. Try 'git bisect" > + " reset <commit>'."), branch.buf); > strbuf_release(&branch); > argv_array_clear(&argv); > - return error(_("could not check out original" > - " HEAD '%s'. Try 'git bisect" > - " reset <commit>'."), branch.buf); > + return -1; The patch looks good. Thanks! Johannes > } > argv_array_clear(&argv); > } > -- > 2.21.0 (Apple Git-122.2) > > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Outreachy] [PATCH] bisect--helper: refer branch.buf before strbuf_release(...) 2019-12-08 19:11 ` Johannes Schindelin @ 2019-12-09 8:15 ` Miriam R. 0 siblings, 0 replies; 3+ messages in thread From: Miriam R. @ 2019-12-09 8:15 UTC (permalink / raw) To: Johannes Schindelin; +Cc: git, Tanushree Tumane, Christian Couder El dom., 8 dic. 2019 a las 20:12, Johannes Schindelin (<Johannes.Schindelin@gmx.de>) escribió: > > Hi Miriam, > > welcome to the Git project! > Thank you! :) > On Sun, 8 Dec 2019, Miriam Rubio wrote: > > > From: Tanushree Tumane <tanushreetumane@gmail.com> > > Is this really a patch authored by Tanushree, or is this a fix written by > you, intended to fix a patch authored by Tanushree? > This is really a patch authored by Tanushree. > If it is the latter, please use a completely new commit message and take > the authorship yourself. > > > Move `error("...%s...", branch.buf);` before `strbuf_release(&branch);` > > to release string buffer `branch` and the memory it used. > > This describes the "what?", but the patch already does that. The commit > message should be more about the "why?". > > In this instance, I believe that the commit message should read more like > this: > > -- snip -- > bisect--helper: avoid free-after-use > > In 5e82c3dd22a (bisect--helper: `bisect_reset` shell function in C, > 2019-01-02), the `git bisect reset` subcommand was ported to C. When the > call to `git checkout` failed, an error message was reported to the > user. > > However, this error message used the `strbuf` that had just been > released already. Let's switch that around: first use it, then release > it. > > Signed-off-by: Miriam Rubio <mirucam@gmail.com> > -- snap -- > Ok! Thank you for your advice with the message. > > > > Mentored-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> > > Mentored-by: Christian Couder <chriscool@tuxfamily.org> > > Signed-off-by: Tanushree Tumane <tanushreetumane@gmail.com> > > Signed-off-by: Miriam Rubio <mirucam@gmail.com> > > --- > > builtin/bisect--helper.c | 7 ++++--- > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c > > index 1fbe156e67..3055b2bb50 100644 > > --- a/builtin/bisect--helper.c > > +++ b/builtin/bisect--helper.c > > @@ -169,11 +169,12 @@ static int bisect_reset(const char *commit) > > > > argv_array_pushl(&argv, "checkout", branch.buf, "--", NULL); > > if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) { > > + error(_("could not check out original" > > + " HEAD '%s'. Try 'git bisect" > > + " reset <commit>'."), branch.buf); > > strbuf_release(&branch); > > argv_array_clear(&argv); > > - return error(_("could not check out original" > > - " HEAD '%s'. Try 'git bisect" > > - " reset <commit>'."), branch.buf); > > + return -1; > > The patch looks good. > Great :). I'll change the commit message and I'll resend the patch. > Thanks! > Johannes Thanks, Miriam > > > } > > argv_array_clear(&argv); > > } > > -- > > 2.21.0 (Apple Git-122.2) > > > > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-09 8:16 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-08 17:28 [Outreachy] [PATCH] bisect--helper: refer branch.buf before strbuf_release(...) Miriam Rubio 2019-12-08 19:11 ` Johannes Schindelin 2019-12-09 8:15 ` Miriam R.
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).