netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
@ 2013-01-17 19:29 Simon Que
  2013-01-17 22:47 ` Peter Korsgaard
  2013-01-18 19:29 ` David Miller
  0 siblings, 2 replies; 8+ messages in thread
From: Simon Que @ 2013-01-17 19:29 UTC (permalink / raw)
  To: jacmet, netdev; +Cc: msb, Simon Que

In two places, tmp is initialized implicitly by being passed as a
pointer during a function call.  However, this is not obvious to the
compiler, which logs a warning.

Signed-off-by: Simon Que <sque@chromium.org>
---
 drivers/net/usb/dm9601.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
index e0433ce..94e716d 100644
--- a/drivers/net/usb/dm9601.c
+++ b/drivers/net/usb/dm9601.c
@@ -199,7 +199,7 @@ static int dm_read_shared_word(struct usbnet *dev, int phy, u8 reg, __le16 *valu
 	dm_write_reg(dev, DM_SHARED_CTRL, phy ? 0xc : 0x4);
 
 	for (i = 0; i < DM_TIMEOUT; i++) {
-		u8 tmp;
+		u8 tmp = 0;
 
 		udelay(1);
 		ret = dm_read_reg(dev, DM_SHARED_CTRL, &tmp);
@@ -242,7 +242,7 @@ static int dm_write_shared_word(struct usbnet *dev, int phy, u8 reg, __le16 valu
 	dm_write_reg(dev, DM_SHARED_CTRL, phy ? 0x1a : 0x12);
 
 	for (i = 0; i < DM_TIMEOUT; i++) {
-		u8 tmp;
+		u8 tmp = 0;
 
 		udelay(1);
 		ret = dm_read_reg(dev, DM_SHARED_CTRL, &tmp);
-- 
1.7.8.6

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

* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
  2013-01-17 19:29 [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning Simon Que
@ 2013-01-17 22:47 ` Peter Korsgaard
  2013-01-17 22:54   ` David Miller
  2013-01-18 19:29 ` David Miller
  1 sibling, 1 reply; 8+ messages in thread
From: Peter Korsgaard @ 2013-01-17 22:47 UTC (permalink / raw)
  To: Simon Que; +Cc: netdev, msb

>>>>> "Simon" == Simon Que <sque@chromium.org> writes:

 Simon> In two places, tmp is initialized implicitly by being passed as a
 Simon> pointer during a function call.  However, this is not obvious to the
 Simon> compiler, which logs a warning.

What warning and what compiler version are you using?


 Simon> Signed-off-by: Simon Que <sque@chromium.org>
 Simon> ---
 Simon>  drivers/net/usb/dm9601.c |    4 ++--
 Simon>  1 files changed, 2 insertions(+), 2 deletions(-)

 Simon> diff --git a/drivers/net/usb/dm9601.c b/drivers/net/usb/dm9601.c
 Simon> index e0433ce..94e716d 100644
 Simon> --- a/drivers/net/usb/dm9601.c
 Simon> +++ b/drivers/net/usb/dm9601.c
 Simon> @@ -199,7 +199,7 @@ static int dm_read_shared_word(struct usbnet *dev, int phy, u8 reg, __le16 *valu
 Simon>  	dm_write_reg(dev, DM_SHARED_CTRL, phy ? 0xc : 0x4);
 
 Simon>  	for (i = 0; i < DM_TIMEOUT; i++) {
 Simon> -		u8 tmp;
 Simon> +		u8 tmp = 0;
 
 Simon>  		udelay(1);
 Simon>  		ret = dm_read_reg(dev, DM_SHARED_CTRL, &tmp);
 Simon> @@ -242,7 +242,7 @@ static int dm_write_shared_word(struct usbnet *dev, int phy, u8 reg, __le16 valu
 Simon>  	dm_write_reg(dev, DM_SHARED_CTRL, phy ? 0x1a : 0x12);
 
 Simon>  	for (i = 0; i < DM_TIMEOUT; i++) {
 Simon> -		u8 tmp;
 Simon> +		u8 tmp = 0;
 
 Simon>  		udelay(1);
 Simon>  		ret = dm_read_reg(dev, DM_SHARED_CTRL, &tmp);
 Simon> -- 
 Simon> 1.7.8.6



-- 
Bye, Peter Korsgaard

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

* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
  2013-01-17 22:47 ` Peter Korsgaard
@ 2013-01-17 22:54   ` David Miller
  2013-01-18  7:37     ` Peter Korsgaard
  0 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2013-01-17 22:54 UTC (permalink / raw)
  To: jacmet; +Cc: sque, netdev, msb

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: Thu, 17 Jan 2013 23:47:58 +0100

>>>>>> "Simon" == Simon Que <sque@chromium.org> writes:
> 
>  Simon> In two places, tmp is initialized implicitly by being passed as a
>  Simon> pointer during a function call.  However, this is not obvious to the
>  Simon> compiler, which logs a warning.
> 
> What warning and what compiler version are you using?

I wouldn't bother asking this, unless it's so that you personally
can reproduce this in the future.

The compiler cannot determine with %100 certainty that tmp is
initialized in all paths.  Any time there are conditional
statements guarding setting and the use, the compiler is going
to err on the side of caution and say that if any of those
paths lead to a use without a set it's going to warn.

Even if it's actually impossible.

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

* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
  2013-01-17 22:54   ` David Miller
@ 2013-01-18  7:37     ` Peter Korsgaard
  2013-01-18  8:03       ` David Miller
  0 siblings, 1 reply; 8+ messages in thread
From: Peter Korsgaard @ 2013-01-18  7:37 UTC (permalink / raw)
  To: David Miller; +Cc: sque, netdev, msb

>>>>> "David" == David Miller <davem@davemloft.net> writes:

 >> What warning and what compiler version are you using?

 David> I wouldn't bother asking this, unless it's so that you personally
 David> can reproduce this in the future.

 David> The compiler cannot determine with %100 certainty that tmp is
 David> initialized in all paths.  Any time there are conditional
 David> statements guarding setting and the use, the compiler is going
 David> to err on the side of caution and say that if any of those
 David> paths lead to a use without a set it's going to warn.

Sure, I just wondered as the code hasn't changed in 5+ years, and the
function that gets called is defined just above, so the compiler should
be able to figure it out.

-- 
Bye, Peter Korsgaard

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

* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
  2013-01-18  7:37     ` Peter Korsgaard
@ 2013-01-18  8:03       ` David Miller
  2013-01-18  8:44         ` Peter Korsgaard
  2013-01-18 10:07         ` David Laight
  0 siblings, 2 replies; 8+ messages in thread
From: David Miller @ 2013-01-18  8:03 UTC (permalink / raw)
  To: jacmet; +Cc: sque, netdev, msb

From: Peter Korsgaard <jacmet@sunsite.dk>
Date: Fri, 18 Jan 2013 08:37:41 +0100

>>>>>> "David" == David Miller <davem@davemloft.net> writes:
> 
>  >> What warning and what compiler version are you using?
> 
>  David> I wouldn't bother asking this, unless it's so that you personally
>  David> can reproduce this in the future.
> 
>  David> The compiler cannot determine with %100 certainty that tmp is
>  David> initialized in all paths.  Any time there are conditional
>  David> statements guarding setting and the use, the compiler is going
>  David> to err on the side of caution and say that if any of those
>  David> paths lead to a use without a set it's going to warn.
> 
> Sure, I just wondered as the code hasn't changed in 5+ years, and the
> function that gets called is defined just above, so the compiler should
> be able to figure it out.

Nothing guarentees that a piece of memory passed into an external
function has it's contents initialized completely by that function.

The compiler can't see what usbnet_read_cmd() does with the memory
behind the 'data' pointer argument.  So it makes the only assumption
it can make, which is that 'tmp' might be uninitialized.

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

* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
  2013-01-18  8:03       ` David Miller
@ 2013-01-18  8:44         ` Peter Korsgaard
  2013-01-18 10:07         ` David Laight
  1 sibling, 0 replies; 8+ messages in thread
From: Peter Korsgaard @ 2013-01-18  8:44 UTC (permalink / raw)
  To: David Miller; +Cc: sque, netdev, msb

>>>>> "David" == David Miller <davem@davemloft.net> writes:

 >> Sure, I just wondered as the code hasn't changed in 5+ years, and the
 >> function that gets called is defined just above, so the compiler should
 >> be able to figure it out.

 David> Nothing guarentees that a piece of memory passed into an external
 David> function has it's contents initialized completely by that function.

 David> The compiler can't see what usbnet_read_cmd() does with the memory
 David> behind the 'data' pointer argument.  So it makes the only assumption
 David> it can make, which is that 'tmp' might be uninitialized.

True, I had forgotten about the usbnet_read_cmd() change (which is the
change causing the warning). With that:

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

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

* RE: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
  2013-01-18  8:03       ` David Miller
  2013-01-18  8:44         ` Peter Korsgaard
@ 2013-01-18 10:07         ` David Laight
  1 sibling, 0 replies; 8+ messages in thread
From: David Laight @ 2013-01-18 10:07 UTC (permalink / raw)
  To: David Miller, jacmet; +Cc: sque, netdev, msb

> > Sure, I just wondered as the code hasn't changed in 5+ years, and the
> > function that gets called is defined just above, so the compiler should
> > be able to figure it out.
> 
> Nothing guarentees that a piece of memory passed into an external
> function has it's contents initialized completely by that function.
> 
> The compiler can't see what usbnet_read_cmd() does with the memory
> behind the 'data' pointer argument.  So it makes the only assumption
> it can make, which is that 'tmp' might be uninitialized.

Possibly the called function is being inlined, then the analysis
knows that some paths don't assign a value.
When it isn't inlined it assumes the value is always modified.

	David

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

* Re: [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning
  2013-01-17 19:29 [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning Simon Que
  2013-01-17 22:47 ` Peter Korsgaard
@ 2013-01-18 19:29 ` David Miller
  1 sibling, 0 replies; 8+ messages in thread
From: David Miller @ 2013-01-18 19:29 UTC (permalink / raw)
  To: sque; +Cc: jacmet, netdev, msb

From: Simon Que <sque@chromium.org>
Date: Thu, 17 Jan 2013 11:29:49 -0800

> In two places, tmp is initialized implicitly by being passed as a
> pointer during a function call.  However, this is not obvious to the
> compiler, which logs a warning.
> 
> Signed-off-by: Simon Que <sque@chromium.org>

Applied to net-next, thanks.

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

end of thread, other threads:[~2013-01-18 19:29 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-17 19:29 [PATCH] net: usb: initialize tmp in dm9601.c to avoid warning Simon Que
2013-01-17 22:47 ` Peter Korsgaard
2013-01-17 22:54   ` David Miller
2013-01-18  7:37     ` Peter Korsgaard
2013-01-18  8:03       ` David Miller
2013-01-18  8:44         ` Peter Korsgaard
2013-01-18 10:07         ` David Laight
2013-01-18 19:29 ` 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).