Re: [LTP] [PATCH] Fix some bashisms

Garrett Cooper
2009-07-06T23:33:41Z

On Mon, Jul 6, 2009 at 3:47 PM, Jiri Palecek<jpalecek@web.de> wrote:
> On Monday 06 July 2009 19:45:08 Garrett Cooper wrote:
>> Jiri,
>>     This patch looks ok for the most part, but here are some comments:
>>
>> 1. if [[ -z "$vnet0" -z "$vnet1" ]] => if [ -z "$vnet0" ] || [ -z "$vnet1" ]
>>
>> It's better read as the following, IMO:
>>
>> [ -z "$vnet0" -o -z "$vnet1" ]
>
> I think this is another "I like this more than that, because this is cool and that sucks" debate. If other think test's "-o" should be used in this case, I would make the change. After all, this can be done by hand directly on the patch...

Why call [(1) twice?

>> 2. All numeric test(1) comparisons could and should be switched from:
>>
>> command
>> if [ $? = 0 ]; then
>>
>> to:
>>
>> if command; then
>>
>> for brevity.
>
> Why should them be changed? This is only a cosmetic change, which would make the patch unnecessarily larger (when it's already a little too large IMHO). If it could be in the former form till now, I think it can stay there a few months/years.
>
> IMHO this usage is still much better than
>
>  command; RC=$?
>  if [ $RC = 0 ] ...
>  ... RC is not read later ...
>
> or (incorrect)
>
>  command || RC=$?
>  if [ $RC = 0 ] ...

Yes, I agree with what you said about the above usage, but if you're
not using $? for other than just the [ $? -ne 0 ], then I wouldn't
even bother doing that, because if command; then saves a fork-exec,
and an additional line in the source. As long as it's readable and
doesn't expand multiple lines with a trailing \, you're fine.

>> 3. About &>
>>
>>    Redirecting Standard Output and Standard Error
>>        Bash allows both the standard output (file descriptor 1) and
>> the standard error output (file descriptor 2) to be redirected to the
>> file whose name is the expansion of word with this construct.
>>
>>        There are two formats for redirecting standard output and standard error:
>>
>>               &>word
>>        and
>>               >&word
>>
>> The output redirection above captures all output (and most of the time
>> you're capturing stdout, but losing stderr to the operating console,
>> which can be undesirable). >& is more portable IIRC (it works with
>> many ash variants, like FreeBSD's sh -- for sure -- and Solaris's sh
>> -- IIRC), which makes it more ideal than &>.
>
> Still, it isn't POSIX, it doesn't work with dash and actually causes pain. See:
>
>  cmd &> fn
>
> POSIX: run cmd in background, write empty file fn
> BASH: run cmd (foreground), send its (normal & error) output to file fn
>
>  cmd >& fn
>
> POSIX: syntax error
> BASH: same as above
>
> But I don't see the point you are making here - do you have a system, where the POSIX way
>
>  cmd > fn 2>&1
>
> doesn't work? Or did you see something else in the patch?

This changes the behavior of the patch though. That's the point ;)...

> Regards
>    Jiri Palecek
>
> BTW: Jiri Palecek <jirka@debian.org> is not my address.

Ok. Your original REPLY-TO email is wonky in your emails though, so I
was playing it by ear...

Thanks,
-Garrett

------------------------------------------------------------------------------
Enter the BlackBerry Developer Challenge
This is your chance to win up to $100,000 in prizes! For a limited time,
vendors submitting new applications to BlackBerry App World(TM) will have
the opportunity to enter the BlackBerry Developer Challenge. See full prize
details at: http://p.sf.net/sfu/blackberry