Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev_put"
From: SF Markus Elfring Date: Sat, 29 Nov 2014 19:00:17 +0100 > From: Markus Elfring > Date: Sat, 29 Nov 2014 18:55:40 +0100 > > The pci_dev_put() function tests whether its argument is NULL > and then returns immediately. Thus the test around the call > is not needed. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring Applied. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call pci_dev_put
From: SF Markus Elfring elfr...@users.sourceforge.net Date: Sat, 29 Nov 2014 19:00:17 +0100 From: Markus Elfring elfr...@users.sourceforge.net Date: Sat, 29 Nov 2014 18:55:40 +0100 The pci_dev_put() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring elfr...@users.sourceforge.net Applied. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev_put"
On Sat, Nov 29, 2014 at 10:00 AM, SF Markus Elfring wrote: > From: Markus Elfring > Date: Sat, 29 Nov 2014 18:55:40 +0100 > > The pci_dev_put() function tests whether its argument is NULL > and then returns immediately. Thus the test around the call > is not needed. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring For this particular case: Acked-by: Olof Johansson Note that the "this might cause problems with backports" case is mostly academic in the scope of _this particular driver_. It's still a very valid discussion and issue though. So I'll be happy to give the ack on this driver, but the larger problem needs consideration still. -Olof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call pci_dev_put
On Sat, Nov 29, 2014 at 10:00 AM, SF Markus Elfring elfr...@users.sourceforge.net wrote: From: Markus Elfring elfr...@users.sourceforge.net Date: Sat, 29 Nov 2014 18:55:40 +0100 The pci_dev_put() function tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring elfr...@users.sourceforge.net For this particular case: Acked-by: Olof Johansson o...@lixom.net Note that the this might cause problems with backports case is mostly academic in the scope of _this particular driver_. It's still a very valid discussion and issue though. So I'll be happy to give the ack on this driver, but the larger problem needs consideration still. -Olof -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev_put"
On 30.11.2014 18:47, Julia Lawall wrote: > > I have heard of at least one case where the problem you are raising > happened in practice... > > julia If one case is known then there are probably a lot more that have not been discovered yet. Maybe this topic should be clarified somewhere (e.g. in "CodingStyle")? On the other hand i always found it obvious that its the callers responsibility to only pass sane parameters to the called functions... Regards, Lino -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev_put"
On Sun, 30 Nov 2014, Lino Sanfilippo wrote: > On 29.11.2014 19:00, SF Markus Elfring wrote: > > > out: > > - if (mac->iob_pdev) > > - pci_dev_put(mac->iob_pdev); > > - if (mac->dma_pdev) > > - pci_dev_put(mac->dma_pdev); > > + pci_dev_put(mac->iob_pdev); > > + pci_dev_put(mac->dma_pdev); > > > > free_netdev(dev); > > out_disable_device: > > > > Hi, > > I know there has been some criticism about those kind of "code > improvements" already but i would like to point out just one more thing: > > Some of those NULL pointer checks on input parameters may have been > added subsequently to functions. So there may be older kernel versions > out there in which those checks dont exists in some cases. If some of > the now "cleaned up" code is backported to such a kernel chances are > good that those missing checks are overseen. And then neither caller nor > callee is doing the NULL pointer check. > > Quite frankly i would vote for the opposite approach: Never rely on the > callee do to checks for NULL and do it always in the caller. An > exception could be for calls on a fast path. But most of those checks > are done on error paths anyway. I have heard of at least one case where the problem you are raising happened in practice... julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call "pci_dev_put"
On 29.11.2014 19:00, SF Markus Elfring wrote: > out: > - if (mac->iob_pdev) > - pci_dev_put(mac->iob_pdev); > - if (mac->dma_pdev) > - pci_dev_put(mac->dma_pdev); > + pci_dev_put(mac->iob_pdev); > + pci_dev_put(mac->dma_pdev); > > free_netdev(dev); > out_disable_device: > Hi, I know there has been some criticism about those kind of "code improvements" already but i would like to point out just one more thing: Some of those NULL pointer checks on input parameters may have been added subsequently to functions. So there may be older kernel versions out there in which those checks dont exists in some cases. If some of the now "cleaned up" code is backported to such a kernel chances are good that those missing checks are overseen. And then neither caller nor callee is doing the NULL pointer check. Quite frankly i would vote for the opposite approach: Never rely on the callee do to checks for NULL and do it always in the caller. An exception could be for calls on a fast path. But most of those checks are done on error paths anyway. Just my 2 cents. Regards, Lino -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call pci_dev_put
On 29.11.2014 19:00, SF Markus Elfring wrote: out: - if (mac-iob_pdev) - pci_dev_put(mac-iob_pdev); - if (mac-dma_pdev) - pci_dev_put(mac-dma_pdev); + pci_dev_put(mac-iob_pdev); + pci_dev_put(mac-dma_pdev); free_netdev(dev); out_disable_device: Hi, I know there has been some criticism about those kind of code improvements already but i would like to point out just one more thing: Some of those NULL pointer checks on input parameters may have been added subsequently to functions. So there may be older kernel versions out there in which those checks dont exists in some cases. If some of the now cleaned up code is backported to such a kernel chances are good that those missing checks are overseen. And then neither caller nor callee is doing the NULL pointer check. Quite frankly i would vote for the opposite approach: Never rely on the callee do to checks for NULL and do it always in the caller. An exception could be for calls on a fast path. But most of those checks are done on error paths anyway. Just my 2 cents. Regards, Lino -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call pci_dev_put
On Sun, 30 Nov 2014, Lino Sanfilippo wrote: On 29.11.2014 19:00, SF Markus Elfring wrote: out: - if (mac-iob_pdev) - pci_dev_put(mac-iob_pdev); - if (mac-dma_pdev) - pci_dev_put(mac-dma_pdev); + pci_dev_put(mac-iob_pdev); + pci_dev_put(mac-dma_pdev); free_netdev(dev); out_disable_device: Hi, I know there has been some criticism about those kind of code improvements already but i would like to point out just one more thing: Some of those NULL pointer checks on input parameters may have been added subsequently to functions. So there may be older kernel versions out there in which those checks dont exists in some cases. If some of the now cleaned up code is backported to such a kernel chances are good that those missing checks are overseen. And then neither caller nor callee is doing the NULL pointer check. Quite frankly i would vote for the opposite approach: Never rely on the callee do to checks for NULL and do it always in the caller. An exception could be for calls on a fast path. But most of those checks are done on error paths anyway. I have heard of at least one case where the problem you are raising happened in practice... julia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] net-PA Semi: Deletion of unnecessary checks before the function call pci_dev_put
On 30.11.2014 18:47, Julia Lawall wrote: I have heard of at least one case where the problem you are raising happened in practice... julia If one case is known then there are probably a lot more that have not been discovered yet. Maybe this topic should be clarified somewhere (e.g. in CodingStyle)? On the other hand i always found it obvious that its the callers responsibility to only pass sane parameters to the called functions... Regards, Lino -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
