>----Original Message----
>From: johannes.schinde...@gmx.de
>Date: 22/12/2017 11:50 
>To: <git@vger.kernel.org>
>Cc: "Junio C Hamano"<gits...@pobox.com>, "Phillip Wood"<phillip.
w...@dunelm.org.uk>, "Kaartic Sivaraam"<kaartic.sivar...@gmail.com>
>Subj: [PATCH] sequencer: assign only free()able strings to gpg_sign
>
>The gpg_sign member of the replay_opts structure is of type `char *`,
>meaning that the sequencer deems the string to which gpg_sign points 
to
>be under its custody, i.e. it needs to be free()d by the sequencer.
>
>Therefore, let's only assign malloc()ed buffers to it.
>
>Reported-by: Kaartic Sivaraam <kaartic.sivar...@gmail.com>
>Signed-off-by: Johannes Schindelin <johannes.schinde...@gmx.de>
>---
>
>       Phillip, if you want to squash these changes into your patches,
>       I'd totally fine with that.
>

Hi Johannes, thanks for putting this together, the patch it fixes is 
already in next so I think it'd be best to leave this one separate. I 
wonder if it would be worth adding another test, see below.

Best Wishes


Phillip

>Based-On: pw/sequencer-in-process-commit at https://github.com/dscho/git

>Fetch-Base-Via: git fetch https://github.com/dscho/git pw/sequencer-
in-process-commit
>Published-As: 
>https://github.com/dscho/git/releases/tag/sequencer-owns-gpg-sign-v1

>Fetch-It-Via: git fetch https://github.com/dscho/git sequencer-owns-
gpg-sign-v1
> sequencer.c                   |  2 +-
> t/t3404-rebase-interactive.sh | 10 ++++++++++
> 2 files changed, 11 insertions(+), 1 deletion(-)
>
>diff --git a/sequencer.c b/sequencer.c
>index 7051b20b762..1b2599668f5 100644
>--- a/sequencer.c
>+++ b/sequencer.c
>@@ -160,7 +160,7 @@ static int git_sequencer_config(const char *k, 
const char *v, void *cb)
>       }
> 
>       if (!strcmp(k, "commit.gpgsign")) {
>-              opts->gpg_sign = git_config_bool(k, v) ? "" : NULL;
>+              opts->gpg_sign = git_config_bool(k, v) ? xstrdup("") : NULL;
>               return 0;
>       }
> 
>diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-
interactive.sh
>index 9ed0a244e6c..040ef1a4dbc 100755
>--- a/t/t3404-rebase-interactive.sh
>+++ b/t/t3404-rebase-interactive.sh
>@@ -1318,6 +1318,16 @@ test_expect_success 'editor saves as CR/LF' '
> 
> SQ="'"
> test_expect_success 'rebase -i --gpg-sign=<key-id>' '
>+      test_when_finished "test_might_fail git rebase --abort" &&
>+      set_fake_editor &&
>+      FAKE_LINES="edit 1" git rebase -i --gpg-sign="\"S I Gner\"" HEAD^ \
>+              >out 2>err &&
>+      test_i18ngrep "$SQ-S\"S I Gner\"$SQ" err
>+'
>+
>+test_expect_success 'rebase -i --gpg-sign=<key-id> overrides commit.
gpgSign' '
>+      test_when_finished "test_might_fail git rebase --abort" &&
>+      test_config commit.gpgsign true &&
>       set_fake_editor &&
>       FAKE_LINES="edit 1" git rebase -i --gpg-sign="\"S I Gner\"" HEAD^ \
>               >out 2>err &&


I thought the bug could be triggered when commit.gpgsign was true and 
it was not overriden on the commandline, is it worth adding a test for 
that?


>base-commit: 28d6daed4f119940ace31e523b3b272d3d153d04
>-- 
>2.15.1.windows.2
>


Reply via email to