From mboxrd@z Thu Jan 1 00:00:00 1970 From: Erik van Konijnenburg Date: Wed, 23 Feb 2005 13:04:47 +0000 Subject: Re: [ANNOUNCE] hotplug-ng 001 release Message-Id: <20050223140447.G2736@banaan.localdomain> List-Id: References: <20050211004033.GA26624@suse.de> In-Reply-To: <20050211004033.GA26624@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: linux-hotplug@vger.kernel.org On Wed, Feb 23, 2005 at 02:40:01PM +0300, Roman Kagan wrote: > 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. OK, I was worrying about migration issues, but the fact that d* matches dl*dh* covers that. > > > > + /* 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. Oops, you're right. > 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? Quoting from glibc-2.3.2.ds1/build-tree/glibc-2.3.2/posix/fnmatch_loop.c: /* We have to handling the symbols differently in ranges since then the collation sequence is important. */ is_range = *p = L('-') && p[1] != L('\0'); That suggests they want locale dictionary order, not hex. I could not find fnmatch in the klibc distributed with hotplug-ng or in old udev sources lying around. A small test program shows some silly matches: konijn@framboos:~/libc/x$ cat t.c; ./t main () { t ("[0-F]", "1"); t ("[0-F]", "D"); t ("[0-9A-F]", "D"); t ("[A-F]", "D"); t ("[0-F]", "?"); t ("[0-F]", "d"); t ("[0-d]", "F"); t ("[0-F]", ":"); t ("[0-9]", ":"); } t (char * pat, char * val) { int matches = ! fnmatch (pat, val, 0); printf ("match pat=%s, val=%s, matches=%d\n", pat, val, matches); } match pat=[0-F], val=1, matches=1 match pat=[0-F], val=D, matches=1 match pat=[0-9A-F], val=D, matches=1 match pat=[A-F], val=D, matches=1 match pat=[0-F], val=?, matches=1 match pat=[0-F], val=d, matches=0 match pat=[0-d], val=F, matches=1 match pat=[0-F], val=:, matches=1 match pat=[0-9], val=:, matches=0 konijn@framboos:~/libc/x$ Regards, Erik ------------------------------------------------------- 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_id396&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