* [Patch] Signedness issue in drivers/net/phy/phy_device.c
@ 2006-08-19 17:33 Eric Sesterhenn
2006-08-20 4:12 ` Richard Knutsson
0 siblings, 1 reply; 6+ messages in thread
From: Eric Sesterhenn @ 2006-08-19 17:33 UTC (permalink / raw)
To: linux-kernel; +Cc: drzeus-sdhci
hi,
while checking gcc 4.1 -Wextra warnings, I stumbled across the following
two warnings:
drivers/net/phy/phy_device.c:528: warning: comparison of unsigned expression < 0 is always false
drivers/net/phy/phy_device.c:546: warning: comparison of unsigned expression < 0 is always false
Since phy_read() returns an integer and can return negative values, it
seems to me the best way to get proper error handling working again
is to make val an int. Currently it is an u32, so the < 0 check
always fails.
Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
--- linux-2.6.18-rc4/drivers/net/phy/phy_device.c.orig 2006-08-19 18:22:56.000000000 +0200
+++ linux-2.6.18-rc4/drivers/net/phy/phy_device.c 2006-08-19 18:24:49.000000000 +0200
@@ -513,7 +513,7 @@ EXPORT_SYMBOL(genphy_read_status);
static int genphy_config_init(struct phy_device *phydev)
{
- u32 val;
+ int val;
u32 features;
/* For now, I'll claim that the generic driver supports
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Patch] Signedness issue in drivers/net/phy/phy_device.c
2006-08-19 17:33 [Patch] Signedness issue in drivers/net/phy/phy_device.c Eric Sesterhenn
@ 2006-08-20 4:12 ` Richard Knutsson
2006-08-20 18:36 ` Eric Sesterhenn / Snakebyte
0 siblings, 1 reply; 6+ messages in thread
From: Richard Knutsson @ 2006-08-20 4:12 UTC (permalink / raw)
To: Eric Sesterhenn; +Cc: linux-kernel, drzeus-sdhci
Eric Sesterhenn wrote:
>hi,
>
>
hello
>while checking gcc 4.1 -Wextra warnings, I stumbled across the following
>two warnings:
>
>drivers/net/phy/phy_device.c:528: warning: comparison of unsigned expression < 0 is always false
>drivers/net/phy/phy_device.c:546: warning: comparison of unsigned expression < 0 is always false
>
>Since phy_read() returns an integer and can return negative values, it
>seems to me the best way to get proper error handling working again
>is to make val an int. Currently it is an u32, so the < 0 check
>always fails.
>
>Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
>
>--- linux-2.6.18-rc4/drivers/net/phy/phy_device.c.orig 2006-08-19 18:22:56.000000000 +0200
>+++ linux-2.6.18-rc4/drivers/net/phy/phy_device.c 2006-08-19 18:24:49.000000000 +0200
>@@ -513,7 +513,7 @@ EXPORT_SYMBOL(genphy_read_status);
>
> static int genphy_config_init(struct phy_device *phydev)
> {
>- u32 val;
>+ int val;
>
>
Would it not be preferable to use a 's32' instead of an 'int'? After
all, it seem 'val' needs to be 32 bits.
Just a thought
/Richard
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Patch] Signedness issue in drivers/net/phy/phy_device.c
2006-08-20 4:12 ` Richard Knutsson
@ 2006-08-20 18:36 ` Eric Sesterhenn / Snakebyte
2006-08-20 19:16 ` Dave Jones
2006-08-20 20:26 ` Richard Knutsson
0 siblings, 2 replies; 6+ messages in thread
From: Eric Sesterhenn / Snakebyte @ 2006-08-20 18:36 UTC (permalink / raw)
To: Richard Knutsson; +Cc: Eric Sesterhenn, linux-kernel, drzeus-sdhci
* Richard Knutsson (ricknu-0@student.ltu.se) wrote:
> Eric Sesterhenn wrote:
hi,
> Would it not be preferable to use a 's32' instead of an 'int'? After
> all, it seem 'val' needs to be 32 bits.
not sure, but wouldnt this collide with platforms where an int is 64
Bits?
Eric
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] Signedness issue in drivers/net/phy/phy_device.c
2006-08-20 18:36 ` Eric Sesterhenn / Snakebyte
@ 2006-08-20 19:16 ` Dave Jones
2006-08-20 20:39 ` Eric Sesterhenn
2006-08-20 20:26 ` Richard Knutsson
1 sibling, 1 reply; 6+ messages in thread
From: Dave Jones @ 2006-08-20 19:16 UTC (permalink / raw)
To: Eric Sesterhenn / Snakebyte; +Cc: Richard Knutsson, linux-kernel, drzeus-sdhci
On Sun, Aug 20, 2006 at 08:36:00PM +0200, Eric Sesterhenn / Snakebyte wrote:
> * Richard Knutsson (ricknu-0@student.ltu.se) wrote:
> > Eric Sesterhenn wrote:
> hi,
>
> > Would it not be preferable to use a 's32' instead of an 'int'? After
> > all, it seem 'val' needs to be 32 bits.
>
> not sure, but wouldnt this collide with platforms where an int is 64
> Bits?
None of the 64-bit Linux ports use ILP64.
Dave
--
http://www.codemonkey.org.uk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] Signedness issue in drivers/net/phy/phy_device.c
2006-08-20 19:16 ` Dave Jones
@ 2006-08-20 20:39 ` Eric Sesterhenn
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sesterhenn @ 2006-08-20 20:39 UTC (permalink / raw)
To: Dave Jones; +Cc: Richard Knutsson, linux-kernel, drzeus-sdhci
On Sun, 2006-08-20 at 15:16 -0400, Dave Jones wrote:
> On Sun, Aug 20, 2006 at 08:36:00PM +0200, Eric Sesterhenn / Snakebyte wrote:
> > * Richard Knutsson (ricknu-0@student.ltu.se) wrote:
> > > Eric Sesterhenn wrote:
> > hi,
> >
> > > Would it not be preferable to use a 's32' instead of an 'int'? After
> > > all, it seem 'val' needs to be 32 bits.
> >
> > not sure, but wouldnt this collide with platforms where an int is 64
> > Bits?
>
> None of the 64-bit Linux ports use ILP64.
Here is an updated patch.
while checking gcc 4.1 -Wextra warnings, I stumbled across the following
two warnings:
drivers/net/phy/phy_device.c:528: warning: comparison of unsigned expression < 0 is always false
drivers/net/phy/phy_device.c:546: warning: comparison of unsigned expression < 0 is always false
Since phy_read() returns an integer and can return negative values, as proposed
by Richard Knutsson this patch changes val to s32. Currently it is an u32, so the < 0 check
always fails.
Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>
--- linux-2.6.18-rc4/drivers/net/phy/phy_device.c.orig 2006-08-20 22:05:26.000000000 +0200
+++ linux-2.6.18-rc4/drivers/net/phy/phy_device.c 2006-08-20 22:05:42.000000000 +0200
@@ -513,7 +513,7 @@ EXPORT_SYMBOL(genphy_read_status);
static int genphy_config_init(struct phy_device *phydev)
{
- u32 val;
+ s32 val;
u32 features;
/* For now, I'll claim that the generic driver supports
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch] Signedness issue in drivers/net/phy/phy_device.c
2006-08-20 18:36 ` Eric Sesterhenn / Snakebyte
2006-08-20 19:16 ` Dave Jones
@ 2006-08-20 20:26 ` Richard Knutsson
1 sibling, 0 replies; 6+ messages in thread
From: Richard Knutsson @ 2006-08-20 20:26 UTC (permalink / raw)
To: Eric Sesterhenn / Snakebyte; +Cc: linux-kernel, drzeus-sdhci
Eric Sesterhenn / Snakebyte wrote:
>* Richard Knutsson (ricknu-0@student.ltu.se) wrote:
>
>
>>Would it not be preferable to use a 's32' instead of an 'int'? After
>>all, it seem 'val' needs to be 32 bits.
>>
>>
>
>not sure, but wouldnt this collide with platforms where an int is 64
>Bits?
>
>
Because of 'phy_read'? It shouldn't as long the returning value is
within the range of the s32.
>Eric
>
>
Richard
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-08-20 20:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-08-19 17:33 [Patch] Signedness issue in drivers/net/phy/phy_device.c Eric Sesterhenn
2006-08-20 4:12 ` Richard Knutsson
2006-08-20 18:36 ` Eric Sesterhenn / Snakebyte
2006-08-20 19:16 ` Dave Jones
2006-08-20 20:39 ` Eric Sesterhenn
2006-08-20 20:26 ` Richard Knutsson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox