--- Begin Message ---
|
Subject: |
[PATCH] dfa: minor fix for whether dfa is fast or not |
|
Date: |
Fri, 05 Aug 2016 20:30:49 +0900 |
dfaoptimize() is not set fast flag even if it is success, but it is wrong.
If success, dfa matcher uses algorithm for single byte, and it is so fast.
I think this bug does not affect for grep, but it will affect with the
patch that I just sent to gawk.
0001-dfa-minor-fix-for-whether-dfa-is-fast-or-not.patch
Description: Binary data
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#24159: [PATCH] dfa: minor fix for whether dfa is fast or not |
|
Date: |
Fri, 5 Aug 2016 22:02:31 -0700 |
On Fri, Aug 5, 2016 at 7:32 PM, Norihiro Tanaka <address@hidden> wrote:
>
> On Fri, 5 Aug 2016 13:29:43 -0700
> Jim Meyering <address@hidden> wrote:
>
>> On Fri, Aug 5, 2016 at 4:30 AM, Norihiro Tanaka <address@hidden> wrote:
>> > dfaoptimize() is not set fast flag even if it is success, but it is wrong.
>> > If success, dfa matcher uses algorithm for single byte, and it is so fast.
>> >
>> > I think this bug does not affect for grep, but it will affect with the
>> > patch that I just sent to gawk.
>>
>> Thank you for the patch.
>> I was going to push it with the attached slightly updated log message.
>> Note however that grep does use that -> fast member via dfasearch.c's
>> use of the dfaisfast function.
>> But then I realized I should at least verify with "make check", and
>> found that this makes grep's dfa-match test fail.
>> Thus, I will not be pushing it as-is.
>
> Thanks for review and adjustment. I re-ran all tests including dfa-match,
> and they were passwd again in my machine. Next, I will re-run them on
> Fedora24, as my machine is RHEL 6.8 and GCC 4.4.7 which is too old.
>
> However, I do not know why dfa-match test fails on your machine.
> dfa-match test does not use grep. It directly calls dfa functions through
> dfa-match-aux executable in order to test codes of dfa which grep does
> not use. dfa-match-aux does not referer to the ->fast member.
I have examined the logs, which suggest it was a false positive in a
parallelized "make check" run, due to that test's 3-second timeout. I
have tried repeatedly to reproduce that failure, so far without
success, but in coreutils development, with parallelized tests, we
fixed many hard-to-reproduce tests with small timeout limits like this
-- most of them now use 10 seconds as the limit, so I will change this
one, too (and several others) with the attached patch.
I have pushed your patch.
0001-tests-standardize-on-10-second-timeouts-to-avoid-rar.diff
Description: Text document
--- End Message ---