Look who's a committer now

[David Zeuthen]

On Sat, 2006-05-13 at 15:19 -0400, Joe Marcus Clarke wrote:
> Here is my first pass at cleaning up tools.  

Thanks.

> The patch adds a freebsd
> subdirectory under tools that stores the OS-dependent portions of the
> scripts.  The tools/ versions of the scripts have been modified to check
> HALD_UNAME_S, and exec the appropriate backend (if available).  I have
> also populated the linux subdirectory with the backend scripts.
> hald_runner.c has been updated to pass utsname.sysname to all scripts
> via the HALD_UNAME_S environment variable (this value is always in
> lowercase).  Finally, hal-storage-mount.c has been updated with code
> from both Jean-Yves and myself to support FreeBSD.
> 
> If this is approved, I will submit an fdi patch.
> 
> http://www.marcuscom.com/downloads/hal.diff

Looks good, only a few comments;

> +++ tools/freebsd/Makefile.am	Sat May 13 14:58:07 2006
> @@ -0,0 +1,17 @@
> +## Process this file with automake to produce Makefile.in
> +
> +transform += ; s,$$,-freebsd,
...
> +
> +script_SCRIPTS =				\
> +	hal-system-power-suspend		\
...


Ought to give the OS specific scripts the -$(HALD_UNAME_S) suffix in the
source directory too; just makes it easier if one is updating multiple
scripts at a time. So in tools/freebsd/, hal-system-power-suspend would
be hal-system-power-suspend-freebsd, in tools/linux it would be
hal-system-power-suspend-linux and so forth.



> +++ tools/freebsd/hal-system-lcd-set-brightness	Sat May 13 14:02:06 2006
...
> +# Check for values outside range
> +if [ ${value} -lt 0 ] || [ ${value} -ge $HAL_PROP_LAPTOP_PANEL_NUM_LEVELS ]; then
> +	echo "org.freedesktop.Hal.Device.LaptopPanel.Invalid" >&2
> +	echo "Brightness has to be between 0 and $HAL_PROP_LAPTOP_PANEL_NUM_LEVELS!" >&2
> +	exit 1
> +fi
> 

This ought to be in tools/hal-system-lcd-set-brightness as it's a check
on general semantics specified by hal.

> +++ tools/freebsd/hal-system-power-suspend	Sat May 13 14:08:49 2006
> @@ -0,0 +1,39 @@
> +#!/bin/sh
> +
> +POWERSAVED_SUSPEND2RAM="dbus-send --system --dest=com.novell.powersave \
> +                        --print-reply /com/novell/powersave \
> +                        com.novell.powersave.action.SuspendToRam"

You probably don't have Novell's powersave stuff on FreeBSD so no need
to carry this over :-)

Apart from that it looks good. Please go ahead and commit it to CVS HEAD
and remember to add entries to the ChangeLog. Thanks!

    David