>----Original Message---- >From: johannes.schindelin@xxxxxx >Date: 22/12/2017 11:50 >To: <git@xxxxxxxxxxxxxxx> >Cc: "Junio C Hamano"<gitster@xxxxxxxxx>, "Phillip Wood"<phillip. wood@xxxxxxxxxxxxx>, "Kaartic Sivaraam"<kaartic.sivaraam@xxxxxxxxx> >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.sivaraam@xxxxxxxxx> >Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> >--- > > 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 >
