* [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).