[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