[Debian-eeepc-devel] package eeepc-acpi-scripts: inefficiencies and inconsistencies
Ben Armstrong
synrg at sanctuary.nslug.ns.ca
Mon Apr 5 01:01:22 UTC 2010
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
More information about the Debian-eeepc-devel
mailing list