Re: [ANNOUNCE] hotplug-ng 001 release

[Roman Kagan <rkagan@mail.ru>]

On Wed, Feb 23, 2005 at 01:44:04AM +0100, Erik van Konijnenburg wrote:
> > -/* Looks like "usb:vNpNdlNdhNdcNdscNdpNicNiscNipN" */
> > -static int do_usb_entry(const char *filename,
> > -			struct usb_device_id *id, char *alias)
> > +/* USB is special because the bcdDevice can be matched against a numeric range */
> > +/* Looks like "usb:vNpNdNdcNdscNdpNicNiscNipN" */
> 
> Hmm.  A change in the alias pattern: dl*dh* => d*, but given the meaning of
> the pattern it's probably the right thing.  Who generates stuff
> that needs to be matched against this?

One of those who did was my patch I sent a few days ago :)

Whoever it was, there were only few trivial cases which worked: when
either USB_DEVICE_ID_MATCH_DEV_LO or USB_DEVICE_ID_MATCH_DEV_HI were
false, or when bcdDevice_lo = bcdDevice_hi.  Otherwise there's no
sensible way to construct something which would match against two
different numbers simultaneously.

BTW the most common match-everything case dl*dh* will be converted into
d* and thus continue to work.

> > +	/* Convert numeric bcdDevice range into fnmatch-able pattern(s) */
> > +	for (ndigits = sizeof(id->bcdDevice_lo) * 2 - 1; 1; ndigits--) {
> 
> By changing the termination condition to devhi < devlo, we could
> avoid ghost matches for ranges such as 6009-4002.

Ah, valid point, thanks!  I didn't test it.

> The loop body is tricky; at first sight it looks OK, but I did not do a full
> blown analysis with loop invariants.

Yep, the code is somewhat ugly, I'll try to clean it up a bit tonight or
tomorrow and resend.

> > +		if (devlo = devhi || !ndigits) {
> > +			if (clo <= chi) {
> > +				sprint_pattern(pattern, ndigits, devlo, clo, chi);
> > +				do_usb_entry(id, pattern, mod);
> > +				printf("%s\n", pattern);
> > +			}
> > +			break;
> > +		}
> > +
> > +		if (clo != 0x0) {
> > +			sprint_pattern(pattern, ndigits, devlo, clo, 0xf);
> > +			do_usb_entry(id, pattern, mod);
> > +			printf("%s\n", pattern);
> > +			devlo++;
> > +		}
> > +
> > +		if (chi != 0xf) {
> > +			sprint_pattern(pattern, ndigits, devhi, 0x0, chi);
> > +			do_usb_entry(id, pattern, mod);
> > +			printf("%s\n", pattern);
> > +			devhi--;
> 
> Signedness issues?  Consider the range 0001-0002: at start of the loop
> body you split in 000 + 1, 000 + 2, here you decrement unsigned devhi
> from 0 to 0xfffffff.

No, when I reach here I have devlo != devhi, with normal ranges (or with
your proposed check added) this means devlo < devhi, so no
over/underflows are possible.

BTW are alphanumeric ranges like [3-C] guaranteed to work with fnmatch
as expected for hex digits regardless of locale settings?  Otherwise
I'll need to replace them with explicit list of allowed digits, i.e.
[3456789ABC] (not any more difficult but less beautiful).  Anybody here
with a clue?

Cheers,
  Roman.


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id\x14396&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel