bug#6547: GNU coreutils 8.5 "make check" => tests/test-suite.log 8 of 365 tests failed

[Pádraig Brady]

On 05/07/10 13:35, Jim Meyering wrote:
> Pádraig Brady wrote:
>> On 04/07/10 11:54, Pádraig Brady wrote:
>> +retry_delay_()
>> +{
>> +  local test_func="$1"
> 
> Please don't use double quotes right after an "=" when the RHS is a
> variable reference.  They're not needed.
> 
>> +  local time_start="$2"
> 
> Maybe `delay', so it doesn't sound like a "starting time"?
> 
>> +  local time_tries="$3"
> 
> `max_n_tries'?
> 
>> +  local attempt=1
>> +  local num_sleeps=$attempt
>> +  local time_fail
>> +  while [ $attempt -le $time_tries ]; do
> 
> We've tried to avoid use of "[...]", and instead use test.
> Slightly less syntax that way, and only one byte longer:
> 
>      while test $attempt -le $time_tries; do
> 
>> +    local delay=$(yes "$time_start" | head -n$num_sleeps | tr '\n' ' ')
> 
> We've had portability problems with yes...|head pipes before, so something
> like this will save a couple processes and should be more portable:
> 
> local delay=$($AWK -v n=$num_sleeps -v s="$delay" \
>               'BEGIN { for (i=0;i<n;i++) t = s" "t; print t }'
> 
>> +    "$1" "$delay" && { time_fail=0; break; } || time_fail=1
> 
> I guess that should be "test_func", rather than "$1"?
> 
>> +    attempt=$(($attempt + 1))
>> +    num_sleeps=$(($num_sleeps * 2))
> 
> These would be the first uses of $((...)).
> I'm pretty sure that construct is not portable enough.
> Please use $(expr ...) instead.
> 
>> +  done
>> +  test "$time_fail" = 0
>> +}

Push with those amendments.

thanks.
Pádraig.