[PATCH] libvolume_id fixes for solaris

Kay Sievers kay.sievers at vrfy.org
Tue Mar 7 07:09:53 PST 2006


On Mon, Mar 06, 2006 at 03:12:57PM -0800, Artem Kachitchkine wrote:
> Kay,
> 
> Could you take a look at this simple patch. Thanks.

Just a note before the comments to the code. I've been asked to clean up
the mess with the several fs guessing code drops on Linux we have today.
This will result in an external use of the vol_id binary provided by udev
on Linux. We will no longer use libvolume_id code inside of HAL.

So you may need to provide an external tool for Solaris too from the time
on we switch over.


> +#ifndef sun
>  	id->type = "vfat";
> +#else
> +	id->type = "pcfs";
> +#endif

> +#ifndef sun
>  	id->type = "iso9660";
> +#else
> +	id->type = "hsfs";
> +#endif

Hmm, the whole goal of HAL is to abstract that system specific information
to applications. Are you really sure you want to do it that way and
patch all the stuff in the upper stack too? (I'm sure no app will recognize
an iPod formatted with "pcfs" :)) Anyway, you better do that "translation"
in the Solaris code then, if you really want to do it. :)

> +#ifndef HAVE_STRNLEN
> +size_t strnlen(const char *s, size_t n)
> +{
> +	char *ptr;
> +
> +	if (s == NULL)
> +		return (n);
> +	ptr = memchr(s, 0, n);
> +	if (ptr == NULL)
> +		return (n);
> +
> +	return ((ptr - s) + 1);
> +}
> +#endif

Just remove the strnlen() use by terminating the initial buffer, that
should do the same trick, right?

> +#ifdef NO_STRUCT_ATTRIBUTE
> +#pragma pack(1)
> +#endif

Can't you compile the whole lib with that pragma if your compiler lacks
support of attributes? It shouldn't make a difference, only the
volume_id struct itself would be packed too, which should be fine. That
way you can do all that from autotools instead of patching all
individial structs with a global compiler instruction.

Anyway, cause in the longer term we will not use that code currently
copied to HAL anymore, I don't mind if you just commit what you need,
but keep in mind that you may need to resolve that in the future as an
external dependency for Solaris too.

Thanks,
Kay


More information about the hal mailing list