netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] qed: add missing header dependencies
@ 2016-09-07 11:07 Baoyou Xie
  2016-09-07 11:37 ` Yuval Mintz
  2016-09-08  0:41 ` David Miller
  0 siblings, 2 replies; 6+ messages in thread
From: Baoyou Xie @ 2016-09-07 11:07 UTC (permalink / raw)
  To: Yuval.Mintz, Ariel.Elior, everest-linux-l2
  Cc: netdev, linux-kernel, arnd, baoyou.xie, xie.baoyou

We get 4 warnings when building kernel with W=1:
drivers/net/ethernet/qlogic/qed/qed_selftest.c:6:5: warning: no previous prototype for 'qed_selftest_memory' [-Wmissing-prototypes]
drivers/net/ethernet/qlogic/qed/qed_selftest.c:19:5: warning: no previous prototype for 'qed_selftest_interrupt' [-Wmissing-prototypes]
drivers/net/ethernet/qlogic/qed/qed_selftest.c:32:5: warning: no previous prototype for 'qed_selftest_register' [-Wmissing-prototypes]
drivers/net/ethernet/qlogic/qed/qed_selftest.c:55:5: warning: no previous prototype for 'qed_selftest_clock' [-Wmissing-prototypes]

In fact, these functions are declared in qed_selftest.h, so this patch
add missing header dependencies.

Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
 drivers/net/ethernet/qlogic/qed/qed_selftest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_selftest.c b/drivers/net/ethernet/qlogic/qed/qed_selftest.c
index a342bfe..9b7678f 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_selftest.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_selftest.c
@@ -2,6 +2,7 @@
 #include "qed_dev_api.h"
 #include "qed_mcp.h"
 #include "qed_sp.h"
+#include "qed_selftest.h"
 
 int qed_selftest_memory(struct qed_dev *cdev)
 {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* RE: [PATCH] qed: add missing header dependencies
  2016-09-07 11:07 [PATCH] qed: add missing header dependencies Baoyou Xie
@ 2016-09-07 11:37 ` Yuval Mintz
  2016-09-07 12:17   ` Arnd Bergmann
  2016-09-08  0:41 ` David Miller
  1 sibling, 1 reply; 6+ messages in thread
From: Yuval Mintz @ 2016-09-07 11:37 UTC (permalink / raw)
  To: Baoyou Xie
  Cc: netdev, linux-kernel, arnd@arndb.de, xie.baoyou@zte.com.cn,
	Ariel Elior

> We get 4 warnings when building kernel with W=1:
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:6:5: warning: no previous
> prototype for 'qed_selftest_memory' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:19:5: warning: no previous
> prototype for 'qed_selftest_interrupt' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:32:5: warning: no previous
> prototype for 'qed_selftest_register' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:55:5: warning: no previous
> prototype for 'qed_selftest_clock' [-Wmissing-prototypes]
> 
> In fact, these functions are declared in qed_selftest.h, so this patch add missing
> header dependencies.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>

While I obviously have no strong objection for including qed_selftest.h
from qed_selftest.c, I'm not sure I understand which C standard dictates
this requirement.
Why should a function definition [not call] be preceded by a prototype?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qed: add missing header dependencies
  2016-09-07 11:37 ` Yuval Mintz
@ 2016-09-07 12:17   ` Arnd Bergmann
  2016-09-07 12:31     ` Yuval Mintz
  0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2016-09-07 12:17 UTC (permalink / raw)
  To: Yuval Mintz
  Cc: Baoyou Xie, netdev, linux-kernel, xie.baoyou@zte.com.cn,
	Ariel Elior

On Wednesday, September 7, 2016 11:37:07 AM CEST Yuval Mintz wrote:
> > We get 4 warnings when building kernel with W=1:
> > drivers/net/ethernet/qlogic/qed/qed_selftest.c:6:5: warning: no previous
> > prototype for 'qed_selftest_memory' [-Wmissing-prototypes]
> > drivers/net/ethernet/qlogic/qed/qed_selftest.c:19:5: warning: no previous
> > prototype for 'qed_selftest_interrupt' [-Wmissing-prototypes]
> > drivers/net/ethernet/qlogic/qed/qed_selftest.c:32:5: warning: no previous
> > prototype for 'qed_selftest_register' [-Wmissing-prototypes]
> > drivers/net/ethernet/qlogic/qed/qed_selftest.c:55:5: warning: no previous
> > prototype for 'qed_selftest_clock' [-Wmissing-prototypes]
> > 
> > In fact, these functions are declared in qed_selftest.h, so this patch add missing
> > header dependencies.
> > 
> > Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> 
> While I obviously have no strong objection for including qed_selftest.h
> from qed_selftest.c, I'm not sure I understand which C standard dictates
> this requirement.
> Why should a function definition [not call] be preceded by a prototype?

This rule addresses two problems:

- some functions should be marked static as they are never used outside
  of the file that declares them. Marking them static give the compiler
  better opportunities for optimization and lets you see when a function
  becomes unused, and if there is no external declaration that is often
  an indication that there are no other users.

- When a function is defined in one file and used in another, you want
  both files to include the same header that has the declaration to
  ensure that the types are identical. There are cases where the
  prototype is changed after the fact in an incompatible way, causing
  silent data corruption on some configurations but maybe not on others.

	Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] qed: add missing header dependencies
  2016-09-07 12:17   ` Arnd Bergmann
@ 2016-09-07 12:31     ` Yuval Mintz
  2016-09-07 12:52       ` Arnd Bergmann
  0 siblings, 1 reply; 6+ messages in thread
From: Yuval Mintz @ 2016-09-07 12:31 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Baoyou Xie, netdev, linux-kernel, xie.baoyou@zte.com.cn,
	Ariel Elior

> > While I obviously have no strong objection for including
> > qed_selftest.h from qed_selftest.c, I'm not sure I understand which C
> > standard dictates this requirement.
> > Why should a function definition [not call] be preceded by a prototype?
> 
> - When a function is defined in one file and used in another, you want
>   both files to include the same header that has the declaration to
>   ensure that the types are identical. There are cases where the
>   prototype is changed after the fact in an incompatible way, causing
>   silent data corruption on some configurations but maybe not on others.

O.k., motivation is clear.
But this really isn't enforced by the ansi-c standard, right?

Anyway, thanks.

Acked-by: Yuval Mintz <Yuval.Mintz@qlogic.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qed: add missing header dependencies
  2016-09-07 12:31     ` Yuval Mintz
@ 2016-09-07 12:52       ` Arnd Bergmann
  0 siblings, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-09-07 12:52 UTC (permalink / raw)
  To: Yuval Mintz
  Cc: Baoyou Xie, netdev, linux-kernel, xie.baoyou@zte.com.cn,
	Ariel Elior

On Wednesday, September 7, 2016 12:31:03 PM CEST Yuval Mintz wrote:
> > > While I obviously have no strong objection for including
> > > qed_selftest.h from qed_selftest.c, I'm not sure I understand which C
> > > standard dictates this requirement.
> > > Why should a function definition [not call] be preceded by a prototype?
> > 
> > - When a function is defined in one file and used in another, you want
> >   both files to include the same header that has the declaration to
> >   ensure that the types are identical. There are cases where the
> >   prototype is changed after the fact in an incompatible way, causing
> >   silent data corruption on some configurations but maybe not on others.
> 
> O.k., motivation is clear.
> But this really isn't enforced by the ansi-c standard, right?

No, ansi-c doesn't enforce this, and even the regular kernel build
flags don't enable the warning in question, we only get it when
either building with "make C=1" using sparse, or "make W=1" to
enable extra warnings from gcc.

The warning is however really useful, and I hope that we go through
all drivers in the kernel and eliminate these warnings in order
to turn them on by default for all drivers.

> Acked-by: Yuval Mintz <Yuval.Mintz@qlogic.com>

Thanks,

	Arnd

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] qed: add missing header dependencies
  2016-09-07 11:07 [PATCH] qed: add missing header dependencies Baoyou Xie
  2016-09-07 11:37 ` Yuval Mintz
@ 2016-09-08  0:41 ` David Miller
  1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2016-09-08  0:41 UTC (permalink / raw)
  To: baoyou.xie
  Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, netdev, linux-kernel,
	arnd, xie.baoyou

From: Baoyou Xie <baoyou.xie@linaro.org>
Date: Wed,  7 Sep 2016 19:07:00 +0800

> We get 4 warnings when building kernel with W=1:
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:6:5: warning: no previous prototype for 'qed_selftest_memory' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:19:5: warning: no previous prototype for 'qed_selftest_interrupt' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:32:5: warning: no previous prototype for 'qed_selftest_register' [-Wmissing-prototypes]
> drivers/net/ethernet/qlogic/qed/qed_selftest.c:55:5: warning: no previous prototype for 'qed_selftest_clock' [-Wmissing-prototypes]
> 
> In fact, these functions are declared in qed_selftest.h, so this patch
> add missing header dependencies.
> 
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>

Applied.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-08  0:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-07 11:07 [PATCH] qed: add missing header dependencies Baoyou Xie
2016-09-07 11:37 ` Yuval Mintz
2016-09-07 12:17   ` Arnd Bergmann
2016-09-07 12:31     ` Yuval Mintz
2016-09-07 12:52       ` Arnd Bergmann
2016-09-08  0:41 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).