On Wed, Aug 27, 2014 at 11:09:08PM +0200, Bjørn Mork wrote: > Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> writes: > > On Wed, Aug 27, 2014 at 09:39:43PM +0200, Ricardo Ribalda Delgado wrote: > >> > >> return sprintf(buf, "pci:v%08Xd%08Xsv%08Xsd%08Xbc%02Xsc%02Xi%02x\n", > > That final 'x' does look like a typo, doesn't it? We are otherwise > consistently using upper-case hex digits for field values and lower case > letter for field names. But it looks like it has been like that since > the beginning, so it might be difficult to fix... Yes, it should be fixed, sorry, my later email said that, no one has hit it in 9+ years, pretty impressive. > > No, the root cause of the problem is a userspace tool looking at a hex > > value as a string and not a number. It doesn't matter if we print it in > > upper or lower case, it's a digit, not a string. Do the numeric > > compare, not a string compare. > > Now I don't really know much about the history here, but the format of > module aliases, using wildcards, seem to suggest a string match to me. > Do you really mean that these strings should be parsed into field names > + values before matching? If that was the intention then surely we > would have exported the fields one-by-one as separate sysfs attributes? > Ref the "one value per file" policy. No, this is a bit field, so you can't do a string compare. kmod should know how do handle this, it does so for other types of "class" fields in module device ids. And no, we didn't export these as a set of files, it's one unique value that you can use to match up a device to a driver. > >> Not many drivers define the pci interface and there is no other driver > >> that has the same vendor and product id. Therefore I see no hurt in > >> adding both patches, one to make the driver broader, and another to > >> fix pci-sysfs. > >> > >> Also, the change on pci-sysfs might affect more stuff and therefore > >> take longer to be applied. > > > > As we have been printing the value to userspace in this way for well > > over a decade now, and nothing has changed, I say it's a userspace bug > > that you should fix instead. Don't work around broken user programs in > > the kernel by changing something that has been stable for 10+ years. > > > > Ok, sorry, not 10+ years, the commit was written May of 2005, so 9 > > years. > > well, just looking at a few common PCI devices on my PCs I wonder if the > reason this hasn't been a problem is because there are _very_ few PCI > programming interfaces using anything by 0-9 digits. One? Looking at the > modules built by Debian I can only find one udc module matching on any > hex value: > > bjorn@nemi:~$ grep pci: /lib/modules/3.16-trunk-amd64/modules.alias|egrep "i[A-F]" > alias pci:v000010DBd00008808sv*sd*bc0Csc03iFE* pch_udc > alias pci:v000010DBd0000801Dsv*sd*bc0Csc03iFE* pch_udc > alias pci:v00008086d00008808sv*sd*bc0Csc03iFE* pch_udc > > This makes me wonder if this is exclusively a problem for PCI UDCs, > which tend to be pretty rare devices? Yes, this is a rare thing, as the age of this line proves :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
