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.
