[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