On 04/04/2010 08:30 PM, Cristian Ionescu-Idbohrn wrote:
Inefficient code (debian/eeepc-acpi-scripts.init):

     # First, get the kernel version.

     KERNEL="`uname -r`"
     case "$KERNEL" in
       2.6.*)
         KERNEL="`echo $KERNEL | sed -re 's/^([0-9]+\.){2}([0-9]+).*$/\2/'`"
         ;;
       *)
         KERNEL=0
         ;;
     esac

Could be written like this:

                # First, get the kernel version.
                read KERNEL<  /proc/version
                KERNEL=${KERNEL#Linux version }
                KERNEL=${KERNEL%%-*}
                case "$KERNEL" in
                        2.6.*)
                                KERNEL=${KERNEL##*.}
                                ;;
                        *)
                                KERNEL=0
                                ;;
                esac

and at least 2 forks avoided.

Inconsistant code (debian/eeepc-acpi-scripts.default.in): why do these
variable values, for example, need to be quoted:

        ENABLE_OSD='no'
        LVDS_OFF='--off'

but not these?

        DETAILED_SOUND_INFO=no
        SUSPEND_OPTIONS=--quirk-s3-bios

Another example (same debian/eeepc-acpi-scripts.init):

        if [ "$KERNEL" -ge 29 -a "`cat /sys/class/dmi/id/product_name`" != 
"900A" ]; then

Why is $KERNEL quoted in an arithmetic comparison, especially when the
code above guarantees the variable value is _not_ empty and integer?

Why is the string 900A quoted?  There are no embedded special characters
in there, AFAICS.

The product_name is picked up in 2 places.  Why not picking it up once,
before entering the if-block, without forking:

        read product_name<  /sys/class/dmi/id/product_name

Why code like this:

       if grep -q '^H.*\brfkill\b' /proc/bus/input/devices; then
         :
       else
         load_module rfkill-input
       fi

if it can be done simplier like this:

        grep -q '^H.*\brfkill\b' /proc/bus/input/devices || load_module 
rfkill-input

Is the 'Debian Eee PC Team' interested in cleanup patches?


Absolutely!

Thanks,
Ben


_______________________________________________
Debian-eeepc-devel mailing list
[email protected]
http://lists.alioth.debian.org/mailman/listinfo/debian-eeepc-devel

Reply via email to