On Sat, 2008-05-17 at 14:32 +0200, Bastian Bittorf wrote:
> hello hackers,
> 
> here are some patches to let the user decide
> wether to use "ifconfig/route..." or the nice
> command "ip", which is much more cleaner IMHO.

Cool.

> This is not complete yet, but let me first know, if
> the used fallback-method or "elegant" or if I have
> to rewrite the code.

I'm not an "official" developer with the openwrt project but I've
written more than my share of shell code.  Just a couple of small style
suggestions...

> Index: package/base-files/files/etc/preinit
> ===================================================================
> --- package/base-files/files/etc/preinit      (Revision 11157)
> +++ package/base-files/files/etc/preinit      (Arbeitskopie)
> @@ -3,8 +3,24 @@
>  export PATH=/bin:/sbin:/usr/bin:/usr/sbin
>  . /etc/diag.sh
>  
> -failsafe_ip() {
> -     ifconfig $ifname 192.168.1.1 netmask 255.255.255.0 broadcast 
> 192.168.1.255 up
> +failsafe_ip ()
> +{
> +     local    DEVICE="$ifname"               # ifname is global known
> +     local        IP="192.168.1.1"
> +     local BROADCAST="192.168.1.255"         # maybe calculate from 
> IP/NETMASK?
> +     local   NETMASK="255.255.255.0"
> +     local CIDR_MASK="24"                    # which must be NETMASK in 
> other notation (maybe calculate?)
                         ^
Don't try to line these up at the "=".  It doesn't increase readability.

> +
> +     ifconfig $DEVICE $IP netmask $NETMASK broadcast $BROADCAST up
> +
> +     [ "$?" -ne 0 ] && {             # returncode not 0 -> ifconfig is n/a 
> -> fallback by using ip
> +
> +             ip addr add dev $DEVICE $IP/$CIDR_MASK broadcast $BROADCAST
> +             ip link set dev $DEVICE arp on  
> +             ip link set dev $DEVICE up
> +     }

Use the || short-circuit syntax:

        ifconfig $DEVICE $IP netmask $NETMASK broadcast $BROADCAST up || {
                # ifconfig failed -> fallback to using ip
                ip addr add dev $DEVICE $IP/$CIDR_MASK broadcast $BROADCAST
                ip link set dev $DEVICE arp on  
                ip link set dev $DEVICE up
        }

b.

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
openwrt-devel mailing list
[email protected]
http://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Reply via email to