| Message ID | 20191209084022.18650-1-mirucam@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [Outreachy] bisect--helper: avoid free-after-use | expand |
Hi Miriam, just a little note on the process: the convention on the Git mailing list is to use `[PATCH v2]`, `[PATCH v3]`, etc when sending revised patch series. It is even available in `git format-patch`'s options: `-v 2` (see https://git-scm.com/docs/git-format-patch#Documentation/git-format-patch.txt--vltngt) On Mon, 9 Dec 2019, Miriam Rubio wrote: > From: Tanushree Tumane <tanushreetumane@gmail.com> > > 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. > > 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> > --- ACK! Thanks, Dscho > 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) > >
Hi Miriam, As Dscho suggests, next time please use [PATCH v2] or [PATCH v3] instead of [PATCH] if the patch has already been sent, even if the subject has changed. On Mon, Dec 9, 2019 at 9:40 AM Miriam Rubio <mirucam@gmail.com> wrote: > > From: Tanushree Tumane <tanushreetumane@gmail.com> > > 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. Great! I think keeping Tanushree as the author and changing the commit message and the subject was the right thing to do. > 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> > --- Here (after the line starting with "---") you can add comments about the patch. One useful comment here would be to say that this patch is a new version of https://public-inbox.org/git/20191208172813.16518-1-mirucam@gmail.com/ which itself has been sent previously by Tanushree (https://public-inbox.org/git/64117cde718f0d56ebfa4c30f4d8fe2155f5cf65.1551003074.git.gitgitgadget@gmail.com/). Thanks, Christian.
El lun., 9 dic. 2019 a las 11:22, Christian Couder (<christian.couder@gmail.com>) escribió: > > Hi Miriam, > > As Dscho suggests, next time please use [PATCH v2] or [PATCH v3] > instead of [PATCH] if the patch has already been sent, even if the > subject has changed. > Ok! I take note. > On Mon, Dec 9, 2019 at 9:40 AM Miriam Rubio <mirucam@gmail.com> wrote: > > > > From: Tanushree Tumane <tanushreetumane@gmail.com> > > > > 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. > > Great! > > I think keeping Tanushree as the author and changing the commit > message and the subject was the right thing to do. > > > 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> > > --- > > Here (after the line starting with "---") you can add comments about > the patch. One useful comment here would be to say that this patch is > a new version of > https://public-inbox.org/git/20191208172813.16518-1-mirucam@gmail.com/ > which itself has been sent previously by Tanushree > (https://public-inbox.org/git/64117cde718f0d56ebfa4c30f4d8fe2155f5cf65.1551003074.git.gitgitgadget@gmail.com/). > Ok, I'll resend another one (v2) adding this comments. :) Thank you. > Thanks, > Christian.
On Mon, Dec 9, 2019 at 11:36 AM Miriam R. <mirucam@gmail.com> wrote: > > > 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> > > > --- > > > > Here (after the line starting with "---") you can add comments about > > the patch. One useful comment here would be to say that this patch is > > a new version of > > https://public-inbox.org/git/20191208172813.16518-1-mirucam@gmail.com/ > > which itself has been sent previously by Tanushree > > (https://public-inbox.org/git/64117cde718f0d56ebfa4c30f4d8fe2155f5cf65.1551003074.git.gitgitgadget@gmail.com/). > > > Ok, I'll resend another one (v2) adding this comments. :) It's not a big deal, but there was no need to resend another one, as the comment is there only to help reviewers with extra information related to where the patch comes from. It will not appear in the commit that will be made from the patch (using `git am`) and I already gave the information about where the patch comes from in my reply to the patch.
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); }