Re: [PATCH 1/3] add testcases for function definitions

[Josh Stone <jistone@redhat.com>]

On 11/09/2015 01:24 PM, David Smith wrote:
> On 11/09/2015 12:07 PM, Josh Stone wrote:
>> On 11/09/2015 12:57 AM, Zhou Wenjian wrote:
>>> +foreach runtime [get_runtime_list] {
>>> +    if {$runtime != ""} {
>>> +	stap_run $srcdir/$subdir/$test.stp no_load ${all_pass_string}${all_pass_string}${all_pass_string}${all_pass_string}${all_pass_string} \
>>> +	    --runtime=$runtime
>>> +    } else {
>>> +	stap_run $srcdir/$subdir/$test.stp no_load ${all_pass_string}${all_pass_string}${all_pass_string}${all_pass_string}${all_pass_string}
>>> +    }
>>> +}
>>
>> I disagree with using repetition like this for "exact" results.  The
>> string already has regex repetition built in:
>>
>>   set all_pass_string "(systemtap test success\r\n)+"
> 
> Ah, I didn't know it did that.
> 
>> '+' means match one or more, greedily.  Repeating this expression on top
>> of itself creates a bad case for the regex engine to backtrack.
>> (It will work, but slowly.)
> 
> Hmm, having repetitions of the all_pass_string *should* work, but
> doesn't. My guess would be that this used to work on older versions of
> tcl/expect, but doesn't work now. The end-of-line handling has always
> been iffy in tcl/expect.

It depends on how things get buffered, and I don't think you can rely on
this to be consistent.

If two success lines get presented at once, then '+' will gobble them
both.  If they're presented one at a time, then '+' will eat one and be
satisfied, then the second line is left unconsumed.

On the other hand, if "(systemtap test success\r\n){2}" gets them
buffered separately, the first time will try and fail to get a complete
match.  Then IIRC expect will append the second line to the buffer and
try all matches again, and "{2}" will now match.

It might be better if we figured out how to process this line-by-line,
instead of a big multiline regex.  We already had to bump stap_run's
exp_match_max to 8192 to cope with this kind of problem.  But any large
change here might have a wide effect across the testsuite callers...


A simpler (untested) idea: stap_run could just add some final output to
anchor the end of the regex, like -E 'probe final { println("EOF") }'.
This can anchor the end of the regex, like "systemtap ending probe" is
anchoring the beginning.  Then little else has to change in the tests.
The '+' above has to be followed by literal EOF or it's not a match.

("probe final" doesn't exist yet, but think opposite of "probe init",
alias "probe final = end(INT64_MAX)", perhaps covering error too.)
((bikeshed: perhaps init/fini are a better pair.))