Re: [PATCH] pickaxe: fix segfault with '-S<...> --pickaxe-regex'

[SZEDER Gábor <szeder.dev@xxxxxxxxx>]

On Sat, Mar 18, 2017 at 4:12 PM, SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote:
> Make sure that the buffer size is reduced on each iteration as the
> buffer pointer is advanced, thus maintaining the correct end of buffer
> location.
>
> The new test is flaky, I've never seen it fail on my Linux box, but
> this is expected according to db5dfa331 (regex: -G<pattern> feeds a
> non NUL-terminated string to regexec() and fails, 2016-09-21).  And
> based on that commit message I would expect the new test without the
> fix to fail reliably on Windows.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
> ---
>
>  diffcore-pickaxe.c      | 5 ++++-
>  t/t4062-diff-pickaxe.sh | 5 +++++
>  2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
> index 9795ca1c1..03f84b714 100644
> --- a/diffcore-pickaxe.c
> +++ b/diffcore-pickaxe.c
> @@ -85,8 +85,11 @@ static unsigned int contains(mmfile_t *mf, regex_t *regexp, kwset_t kws)
>                        !regexec_buf(regexp, data, sz, 1, &regmatch, flags)) {
>                         flags |= REG_NOTBOL;
>                         data += regmatch.rm_eo;
> -                       if (*data && regmatch.rm_so == regmatch.rm_eo)
> +                       sz -= regmatch.rm_eo;
> +                       if (*data && regmatch.rm_so == regmatch.rm_eo) {
>                                 data++;
> +                               sz--;
> +                       }
>                         cnt++;
>                 }
>
> diff --git a/t/t4062-diff-pickaxe.sh b/t/t4062-diff-pickaxe.sh
> index f0bf50bda..7c4903f49 100755
> --- a/t/t4062-diff-pickaxe.sh
> +++ b/t/t4062-diff-pickaxe.sh
> @@ -19,4 +19,9 @@ test_expect_success '-G matches' '
>         test 4096-zeroes.txt = "$(cat out)"
>  '
>
> +test_expect_success '-S --pickaxe-regex' '
> +       git diff --name-only -S0 --pickaxe-regex HEAD^ >out &&
> +       verbose test 4096-zeroes.txt = "$(cat out)"
> +'
> +
>  test_done

Hang on, this new test does fail because of a segfault _with_ the fix
on Travis 64bit Linux and OSX builds.

Oh, well.