On Tue, 07 Jul 2009 02:31:25 +0200, Garrett Cooper <[email protected]> wrote:
> 2009/7/6 Jiří Paleček <[email protected]>: >> On Tue, 07 Jul 2009 01:33:29 +0200, Garrett Cooper <[email protected]> >> wrote: >> >>> 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: >>>>> >>>>> 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. >> >> Are you talking only about efficiency and bash scripts? :-) > > Efficiency, and more importantly readability. > >> Anyway, if you want to save the fork-exec, you can just use a shell >> with a >> test builtin (ksh, bash, dash ... and I believe many others do). >> >> If you are concerned about readability, I suggest considering another >> alternative: >> >> command || { >> do_something_on_error >> } >> >> command && { >> do_something_when_ok >> } >> >> Of course, usable only when there's no "else" branch. > > That's ugly and hard to read with large logic blocks. I don't think so; I have no problems with a similar syntax in C, moreover, I consider the braces more distinguishable than keywords. >> Still, I think the original form is correct and quite straightforward. I >> would probably not use the extra test myself, but I don't feel like it >> needs >> changing. Just looking at the patch, I see places where the check is >> done >> really suspiciously: >> >> >> tst_resm TINFO "Test #1: changing mtu size of eth0:1 device." >> >> - ip link set eth0:1 mtu 300 &>$LTPTMP/tst_ip.err >> + ip link set eth0:1 mtu 300 >$LTPTMP/tst_ip.err 2>&1 >> if [ $RC -ne 0 ] >> then >> tst_res TFAIL $LTPTMP/tst_ip.err \ >> >> I think this needs much more attention than "calling [ twice" or "extra >> line >> in the script". > > Yeah, true. Why not run everything through set -u then? It would not help, IMO. At least in this particular case, the variable is properly initialized. Regards Jiri Palecek ------------------------------------------------------------------------------ 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
