| Message ID | 1358450989-25205-1-git-send-email-sque@chromium.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
>>>>> "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
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> "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.
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> "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>
> > 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 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);
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(-)