Hi !
In 2.6.15, platform device matching works according to this comment in
the code, or rather are supposed to:
* Platform device IDs are assumed to be encoded like this:
* "<name><instance>", where <name> is a short description of the
* type of device, like "pci" or "floppy", and <instance> is the
* enumerated instance of the device, like '0' or '42'.
However, looking a few lines below, I see the actual implemetation:
static int platform_match(struct device * dev, struct device_driver * drv)
{
struct platform_device *pdev = container_of(dev, struct platform_device, dev);
return (strncmp(pdev->name, drv->name, BUS_ID_SIZE) == 0);
}
As far as I know, strncmp() is _NOT_ supposed to return 0 if one string
is shorter than the other and they match until that point. Thus the
above will never match unless the <name> portion of pdev->name is
exactly of size BUS_ID_SIZE which is obviously not the case...
Did I miss something or do we expect a "special" semantic for strncmp in
the kernel ?
Ben.
On Fri, 2006-01-06 at 16:59 +1100, Benjamin Herrenschmidt wrote:
> Hi !
>
> In 2.6.15, platform device matching works according to this comment in
> the code, or rather are supposed to:
.../...
Just in case my analysis is correct, here's an untested fix:
---
Platform device matching doesn't work when an "id" is used.
Signed-off-by: Benjamin Herrenschmidt <[email protected]>
--- linux-work.orig/drivers/base/platform.c 2005-11-24 17:18:43.000000000 +1100
+++ linux-work/drivers/base/platform.c 2006-01-06 16:59:59.000000000 +1100
@@ -447,7 +447,8 @@ static int platform_match(struct device
{
struct platform_device *pdev = container_of(dev, struct platform_device, dev);
- return (strncmp(pdev->name, drv->name, BUS_ID_SIZE) == 0);
+ return strncmp(pdev->name, drv->name,
+ min(BUS_ID_SIZE, strlen(drv->name))) == 0;
}
static int platform_suspend(struct device * dev, pm_message_t state)
On Fri, Jan 06, 2006 at 04:59:39PM +1100, Benjamin Herrenschmidt wrote:
> static int platform_match(struct device * dev, struct device_driver * drv)
> {
> struct platform_device *pdev = container_of(dev, struct platform_device, dev);
>
> return (strncmp(pdev->name, drv->name, BUS_ID_SIZE) == 0);
> }
>
> As far as I know, strncmp() is _NOT_ supposed to return 0 if one string
> is shorter than the other and they match until that point. Thus the
> above will never match unless the <name> portion of pdev->name is
> exactly of size BUS_ID_SIZE which is obviously not the case...
pdev->name is just the <name> part - it's pdev->dev.name which has
both the <name> and <instance>. I think the strncmp is unnecessary,
and it can be replaced by a plain strcmp.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core
On Fri, 2006-01-06 at 18:53 +0000, Russell King wrote:
> On Fri, Jan 06, 2006 at 04:59:39PM +1100, Benjamin Herrenschmidt wrote:
> > static int platform_match(struct device * dev, struct device_driver * drv)
> > {
> > struct platform_device *pdev = container_of(dev, struct platform_device, dev);
> >
> > return (strncmp(pdev->name, drv->name, BUS_ID_SIZE) == 0);
> > }
> >
> > As far as I know, strncmp() is _NOT_ supposed to return 0 if one string
> > is shorter than the other and they match until that point. Thus the
> > above will never match unless the <name> portion of pdev->name is
> > exactly of size BUS_ID_SIZE which is obviously not the case...
>
> pdev->name is just the <name> part - it's pdev->dev.name which has
> both the <name> and <instance>. I think the strncmp is unnecessary,
> and it can be replaced by a plain strcmp.
Ah indeed, I got confused by having both strings.
Ben.
On Fri, 2006-01-06 16:59:39, Benjamin Herrenschmidt wrote:
> In 2.6.15, platform device matching works according to this comment in
> the code, or rather are supposed to:
>
> * Platform device IDs are assumed to be encoded like this:
> * "<name><instance>", where <name> is a short description of the
> * type of device, like "pci" or "floppy", and <instance> is the
> * enumerated instance of the device, like '0' or '42'.
>
> However, looking a few lines below, I see the actual implemetation:
>
> static int platform_match(struct device * dev, struct device_driver * drv)
> {
> struct platform_device *pdev = container_of(dev, struct platform_device, dev);
>
> return (strncmp(pdev->name, drv->name, BUS_ID_SIZE) == 0);
> }
>
> As far as I know, strncmp() is _NOT_ supposed to return 0 if one string
> is shorter than the other and they match until that point. Thus the
> above will never match unless the <name> portion of pdev->name is
> exactly of size BUS_ID_SIZE which is obviously not the case...
>
> Did I miss something or do we expect a "special" semantic for strncmp in
> the kernel ?
I can't speak to the correctness of that code but your understanding of
strncmp() is incorrect. From "GNU C Library Application Fundamentals":
This function is the [sic] similar to strcmp, except that no more
than size wide characters are compared. In other words, if the two
strings are the same in their first size wide characters, the return
value is zero.
And this has been may experience for the past 20 years and is confirmed by
this trivial program which prints zero in both cases:
#include <string.h>
#include <stdio.h>
int main() {
printf("%d\n", strncmp("abc","abcd",3));
printf("%d\n", strncmp("abcd","abc",3));
}
On 1/7/06, Kurtis D. Rader <[email protected]> wrote:
> On Fri, 2006-01-06 16:59:39, Benjamin Herrenschmidt wrote:
> > As far as I know, strncmp() is _NOT_ supposed to return 0 if one string
> > is shorter than the other and they match until that point
> I can't speak to the correctness of that code but your understanding of
> strncmp() is incorrect. From "GNU C Library Application Fundamentals":
I believe the original poster was asserting that
'strncmp("abc","abcd",100) should never return zero,' and not
'strncmp("abc","abcd",3) should never return zero.' Though I also
found the statement confusing at first.
-Bob
> I can't speak to the correctness of that code but your understanding of
> strncmp() is incorrect. From "GNU C Library Application Fundamentals":
>
> This function is the [sic] similar to strcmp, except that no more
> than size wide characters are compared. In other words, if the two
> strings are the same in their first size wide characters, the return
> value is zero.
>
> And this has been may experience for the past 20 years and is confirmed by
> this trivial program which prints zero in both cases:
>
> #include <string.h>
> #include <stdio.h>
> int main() {
> printf("%d\n", strncmp("abc","abcd",3));
> printf("%d\n", strncmp("abcd","abc",3));
> }
Except that in my example, the count was larger than both strings...
Anyway, it happens to not be a problem as the string I was comparing was
not in fact different.
Ben.