[PATCH] preliminary volume formatting support

[David Zeuthen]

Hey,

sorry for the delay,

On Wed, 2006-01-18 at 22:59 -0500, Christopher Santero wrote:
> Here is the beginning of the formatting support. It exposes the Format
> method with signature ss (filesystem type, label) on
> o.fd.H.Device.Volume.Unstable. Right now it is very rudimentary, as
> all it supports is basic formatting, without allowing the user to
> specify options to the various mount commands. Also, only root can
> invoke the method.

Looks good so far, a few comments

> +# For now, only root can format
> +if [ $HAL_METHOD_INVOKED_BY_UID != 0 ] ; then
> +	echo "Only root is allowed to format volumes using this tool."
> +	exit 1
> +fi

You need to pass the exception name first, e.g. 

	echo "org.freedesktop.Hal.Device.Volume.Unstable.PermissionDenied" >&2
	echo "Only root is allowed to format volumes using this tool."
	exit 1

instead. This comment applies other places in your script too.

Also here

> +# Read parameters
> +read FS_TYPE
> +read LABEL

you really really want to guard against a malicious caller passing
`/path/to/some/binary` as you expand $FS_TYPE below. See for example
tools/hal-system-storage-mount on how we guard against this - yes, both
Kay and I did the same mistake with the mounting scripts..

> +		FORMAT_CMD="/sbin/mkfs.ext2 $HAL_PROP_BLOCK_DEVICE"

Suggest to just use mkfs.ext2 as we control $PATH when launching hald;
this makes it easier if some distribution puts mkfs.ext2 in strange
places. Oh, and this comment applies other places in your script too.

Cheers,
David