HEADS UP: More HAL testers needed

[Joe Marcus Clarke]

On Fri, 2006-11-03 at 11:04 +0100, Jean-Yves Lefort wrote:
> On Thu, 02 Nov 2006 18:04:17 -0500
> Joe Marcus Clarke <marcus at FreeBSD.org> wrote:
> 
> > I am working to sync our HAL backend up with the latest HAL spec.  To
> > that end, I have added support for displaying more volume.partition
> > properties.  This patch will only affect HAL objects that have the
> > volume.is_partition property set to TRUE.  The only way you'd notice any
> > difference is using lshal.  The old output looked something like:
> >
> > volume.partition.number = 3 (0x3) (int)
> > volume.msdos_part_table_type = 165 (0xa5) (int)
> >
> > The new style looks like this:
> >
> > volume.partition.start = 18268346880  (0x440e0da00)  (uint64)
> > volume.partition.media_size = 61730726400  (0xe5f702200)  (uint64)
> > volume.partition.type = '0xa5'  (string)
> > volume.partition.scheme = 'mbr'  (string)
> > volume.partition.number = 3  (0x3)  (int)
> >
> > I'd be particularly interested with feedback from those that have gpt or
> > apple schemes, but feedback from mbr users is just as good (i.e. to let
> > me know it works on more that just my system).  Thanks.
> >
> > http://www.marcuscom.com/downloads/patch-hal_volume
> 
> Useless elements underlined:
> 
> +  if (geom_obj->uuid)
>    ^^^^^^^^^^^^^^^^^^^
> +    g_free(geom_obj->uuid);
> +
> +  if (geom_obj->apple_type)
>    ^^^^^^^^^^^^^^^^^^^^^^^^^
> +    g_free(geom_obj->apple_type);

I keep doing that!  I have a big problem freeing a NULL value.

> 
> +  *mediasize = (guint64) hf_probe_volume_getenv_uintmax("HF_VOLUME_SIZE");
>                 ^^^^^^^^^
> +  *offset = (guint64) hf_probe_volume_getenv_uintmax("HF_VOLUME_OFFSET");
>              ^^^^^^^^^
> 
> +  if ((sscanf(partno + 1, "%i", number)) != (int) (strlen(partno) - 1))
>        ^                                ^    ^^^^^ ^                  ^
> 
> Btw, that last construct breaks in two cases:
> 
>   - "partno + 1" is empty: number is left undefined
>   - "partno + 1" is a number > 9: sscanf(...) = 1, strlen > 1
> 
> Do this instead:
> 
>   int len;
>   ...
>   len = strlen(partno) - 1;
>   if (len > 0 && strspn(partno + 1, "0123456789") == len)
>     number = atoi(partno);
>   else
>     return FALSE;

Thanks.

Joe

-- 
Joe Marcus Clarke
FreeBSD GNOME Team      ::      gnome at FreeBSD.org
FreeNode / #freebsd-gnome
http://www.FreeBSD.org/gnome
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: This is a digitally signed message part
Url : http://lists.freebsd.org/pipermail/freebsd-gnome/attachments/20061103/6e74068d/attachment.pgp