List supported burn speeds for CD writers

Danny Kukawka danny.kukawka at web.de
Mon Jan 16 09:11:38 PST 2006


On Monday 16 January 2006 17:04, Ryan Lortie wrote:
> > > 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 thought you was subscriped to the list and after no reaction on my reply 
last week I thought this is o.k. and I asked David for this patch to 
integrate this in next release today.

> 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).  

I don't see that this is easier than a strlist.
 
> It's also less overhead to store and transmit.  

IMO not really overhead, if you not have some hundreds of CD/DVD Burner in 
your system ;-) ... there are other bottle necks in HAL.

> 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.  

No, this could not happen, because this key is only generated if the device is 
added and never again. Because of this the key is always empty and if you 
remove the device and plugin it again ... it's again a empty property, IIRC 
no cashing here.

> 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().

Good to know, but strsep() is not really so much better ;-) : 
BUGS: This function suffers from the same problems as strtok().  In 
particular, it modifies the original string. Avoid it. 

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

If you think ... 

> 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.

Let's to this ...

> 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).

... but a comma-seperated string is IMO also not really better. And with a 
strlist you can easy append/prepend missing values if something wrong 
detected ...  

... if this is what's you make happy ... but please do this now, because we 
would release 0.5.6 in the next hours.

> 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:)

See above.

Cheers,

Danny


More information about the hal mailing list