On Tuesday 19 February 2008 05:00:43 pm Rene Herman wrote:
> On 19-02-08 23:49, Adrian Bunk wrote:
>
> > The Coverity checker spotted the following inconsequent NULL checking
> > introduced by commit 5d38998ed15b31f524bde9a193d60150af30d916:
> >
> > <-- snip -->
> >
> > ...
> > static int pnp_bus_resume(struct device *dev)
> > {
> > ...
> > if (pnp_dev->protocol && pnp_dev->protocol->resume)
> > pnp_dev->protocol->resume(pnp_dev);
> >
> > if (pnp_can_write(pnp_dev)) {
> > ...
> >
> > <-- snip -->
> >
> > pnp_can_write(pnp_dev) dereferences pnp_dev->protocol.
>
> I see, thanks. pnp_bus_suspend() has the same problem (I added this test to
> complete the mirror in fact) and/but is not a real problem since the tests
> are also the first things done inside the blocks they protect -- if
> pnp_dev->protocol isn't set here, we're dead anyway therefore.
>
> That probably means we can just delete the pnp_dev->protocol tests but this
> would need an ack from for example Bjorn Helgaas who might have an idea
> about how generically useful this is designed to be. The no brain thing to
> do would be just as per attached.
I agree with you that we can just delete the dev->protocol tests
completely. So I'd rather see something like this (built but untested):
PNP: remove dev->protocol NULL checks
Every PNP device should have a valid protocol pointer. If it doesn't,
something's wrong and we should oops so we can find and fix the problem.
Signed-off-by: Bjorn Helgaas <[EMAIL PROTECTED]>
Index: work6/drivers/pnp/driver.c
===================================================================
--- work6.orig/drivers/pnp/driver.c 2008-02-20 09:46:01.000000000 -0700
+++ work6/drivers/pnp/driver.c 2008-02-20 09:46:28.000000000 -0700
@@ -167,7 +167,7 @@
return error;
}
- if (pnp_dev->protocol && pnp_dev->protocol->suspend)
+ if (pnp_dev->protocol->suspend)
pnp_dev->protocol->suspend(pnp_dev, state);
return 0;
}
@@ -181,7 +181,7 @@
if (!pnp_drv)
return 0;
- if (pnp_dev->protocol && pnp_dev->protocol->resume)
+ if (pnp_dev->protocol->resume)
pnp_dev->protocol->resume(pnp_dev);
if (pnp_can_write(pnp_dev)) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
