[Debian-eeepc-devel] package eeepc-acpi-scripts: inefficiencies and inconsistencies
Cristian Ionescu-Idbohrn
cristian.ionescu-idbohrn at axis.com
Sun Apr 4 23:30:26 UTC 2010
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?
Cheers,
--
Cristian
More information about the Debian-eeepc-devel
mailing list