List supported burn speeds for CD writers

Ryan Lortie desrt at desrt.ca
Mon Jan 16 08:04:53 PST 2006


On Mon, 2006-16-01 at 10:41 -0500, Danny Kukawka wrote:
> > > I had to do it this way since hal doesn't support the concept of lists
> > > of integers and the libhal interface doesn't even allow you to use lists
> > > of strings (in any case, I believe a comma-separated string to be better
> > > than a list of strings - far less overhead).
> >
> > This is not right. We have already list of strings: strlist. You can use
> > them instead of a comma.seperated string.
> 
> I attached a new version of the patch with strlist instead of comma-separated 
> string. Also updated the spec for this new key.

I missed this email due to being taken off the cc: but David has sent me
a copy of it so now I'll reply.

I don't like this approach.  As noted in my original email (still quoted
^^) I explicitly decided that using a string would be easier than using
a strlist (and easier for the end-user too).  It's also less overhead to
store and transmit.  Even your code uses the comma-separated form and
merely converts it to a list (as opposed to generating a list in the
first place).

The patch itself also does a few unfortunate things (that I've noticed
-- there may be others).  It calls strlist_append without first clearing
the list so if anything was already there before then you'll get
duplicated entries.  It also uses strtok() a couple of times.  From the
strtok manpage: "Avoid using these functions.".  We're not a threaded
program, but since this code is in linux2/ we should take advantage of
the fact that Linux has strsep().

But really, we should just use a single comma-separated string. :)

This isn't ideal but it's best for now.  I've talked to David about the
issue of having better types (for example, integer lists) and it will
happen 'someday' but not for now.  At that point we will be breaking
API/ABI anyway and can change this to the more sensible datatype of
integer list.

I advocate reversing this patch and including my own patch verbatim
until such a time as we have a richer type system.  Using strlists just
because they are "lists" is not the best thing to do here (particularly
since the data in this case is static).

I did forget to modify the spec file when I made my original patch and I
can do that today.

Comments appreciated (+ please keep me on the cc:)

Cheers :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 701 bytes
Desc: This is a digitally signed message part
Url : http://lists.freedesktop.org/archives/hal/attachments/20060116/1d4fa8ba/attachment.pgp


More information about the hal mailing list