On Mon, Jul 6, 2009 at 3:47 PM, Jiri Palecek<[email protected]> 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 <[email protected]> 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 _______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
