[KJ] [Patch] Check unsigned < 0 in /drivers/net/acenic.c

[KJ] [Patch] Check unsigned < 0 in /drivers/net/acenic.c

[Eric Sesterhenn / snakebyte]

[-- Attachment #1: Type: text/plain, Size: 669 bytes --]

hi,

since read_eeprom_byte() returns a negative value on an error
condition, tmp should be signed so the comparisons in line 1007
and 1017 can catch the error. 

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux-2.6.15-rc5-git5/drivers/net/acenic.c.orig	2005-12-16 01:01:44.000000000 +0100
+++ linux-2.6.15-rc5-git5/drivers/net/acenic.c	2005-12-16 01:01:58.000000000 +0100
@@ -905,7 +905,8 @@ static int __devinit ace_init(struct net
 	struct pci_dev *pdev;
 	unsigned long myjif;
 	u64 tmp_ptr;
-	u32 tig_ver, mac1, mac2, tmp, pci_state;
+	u32 tig_ver, mac1, mac2, pci_state;
+	int tmp;
 	int board_idx, ecode = 0;
 	short i;
 	unsigned char cache_size;



[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [Patch] Check unsigned < 0 in /drivers/net/acenic.c

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 671 bytes --]

> since read_eeprom_byte() returns a negative value on an error
> condition, tmp should be signed so the comparisons in line 1007
> and 1017 can catch the error.

> --- linux-2.6.15-rc5-git5/drivers/net/acenic.c.orig
> +++ linux-2.6.15-rc5-git5/drivers/net/acenic.c
> @@ -905,7 +905,8 @@ static int __devinit ace_init(struct net
>  	struct pci_dev *pdev;
>  	unsigned long myjif;
>  	u64 tmp_ptr;
> -	u32 tig_ver, mac1, mac2, tmp, pci_state;
> +	u32 tig_ver, mac1, mac2, pci_state;
> +	int tmp;
>  	int board_idx, ecode = 0;
>  	short i;
>  	unsigned char cache_size;

Looks like there are two different tmps: one for read_eeprom_byte,
another for writing to PCI state.


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [Patch] Check unsigned < 0 in /drivers/net/acenic.c

[Eric Sesterhenn / snakebyte]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

On Mon, 2006-01-09 at 00:15 +0300, Alexey Dobriyan wrote:
> Looks like there are two different tmps: one for read_eeprom_byte,
> another for writing to PCI state.

guess, this patch is better then. but maybe you can suggest
a better variable name then tmp2... :)

Signed-off-by: Eric Sesterhenn <snakebyte@gmx.de>

--- linux-2.6.15/drivers/net/acenic.c.orig	2006-01-09 15:22:53.000000000 +0100
+++ linux-2.6.15/drivers/net/acenic.c	2006-01-09 15:23:55.000000000 +0100
@@ -906,7 +906,7 @@ static int __devinit ace_init(struct net
 	unsigned long myjif;
 	u64 tmp_ptr;
 	u32 tig_ver, mac1, mac2, tmp, pci_state;
-	int board_idx, ecode = 0;
+	int board_idx, tmp2, ecode = 0;
 	short i;
 	unsigned char cache_size;
 
@@ -1003,22 +1003,22 @@ static int __devinit ace_init(struct net
 	mac1 = 0;
 	for(i = 0; i < 4; i++) {
 		mac1 = mac1 << 8;
-		tmp = read_eeprom_byte(dev, 0x8c+i);
-		if (tmp < 0) {
+		tmp2 = read_eeprom_byte(dev, 0x8c+i);
+		if (tmp2 < 0) {
 			ecode = -EIO;
 			goto init_error;
 		} else
-			mac1 |= (tmp & 0xff);
+			mac1 |= (tmp2 & 0xff);
 	}
 	mac2 = 0;
 	for(i = 4; i < 8; i++) {
 		mac2 = mac2 << 8;
-		tmp = read_eeprom_byte(dev, 0x8c+i);
-		if (tmp < 0) {
+		tmp2 = read_eeprom_byte(dev, 0x8c+i);
+		if (tmp2 < 0) {
 			ecode = -EIO;
 			goto init_error;
 		} else
-			mac2 |= (tmp & 0xff);
+			mac2 |= (tmp2 & 0xff);
 	}
 
 	writel(mac1, &regs->MacAddrHi);



[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors

Re: [KJ] [Patch] Check unsigned < 0 in /drivers/net/acenic.c

[Alexey Dobriyan]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

On Mon, Jan 09, 2006 at 03:25:21PM +0100, Eric Sesterhenn / snakebyte wrote:
> On Mon, 2006-01-09 at 00:15 +0300, Alexey Dobriyan wrote:
> > Looks like there are two different tmps: one for read_eeprom_byte,
> > another for writing to PCI state.
>
> guess, this patch is better then. but maybe you can suggest
> a better variable name then tmp2... :)

Argh... Sorry for not being clear enough. "tmp" is a good name as soon
as you put it in those mac reading loops. I fixed it locally.

--- linux-kj.orig/drivers/net/acenic.c
+++ linux-kj/drivers/net/acenic.c
@@ -1002,6 +1002,8 @@ static int __devinit ace_init(struct net
 
 	mac1 = 0;
 	for(i = 0; i < 4; i++) {
+		int tmp;
+
 		mac1 = mac1 << 8;
 		tmp = read_eeprom_byte(dev, 0x8c+i);
 		if (tmp < 0) {
@@ -1012,6 +1014,8 @@ static int __devinit ace_init(struct net
 	}
 	mac2 = 0;
 	for(i = 4; i < 8; i++) {
+		int tmp;
+
 		mac2 = mac2 << 8;
 		tmp = read_eeprom_byte(dev, 0x8c+i);
 		if (tmp < 0) {

> --- linux-2.6.15/drivers/net/acenic.c.orig
> +++ linux-2.6.15/drivers/net/acenic.c	2006-01-09 15:23:55.000000000 +0100
> @@ -906,7 +906,7 @@ static int __devinit ace_init(struct net
>  	unsigned long myjif;
>  	u64 tmp_ptr;
>  	u32 tig_ver, mac1, mac2, tmp, pci_state;
> -	int board_idx, ecode = 0;
> +	int board_idx, tmp2, ecode = 0;
>  	short i;
>  	unsigned char cache_size;


[-- Attachment #2: Type: text/plain, Size: 168 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors