[PATCH] pmc551: fix signedness bug in init_pmc551()

[PATCH] pmc551: fix signedness bug in init_pmc551()

[Xi Wang]

Since "length" is a u32, the error handling below didn't work when
fixup_pmc551() returns -ENODEV.

	if ((length = fixup_pmc551(PCI_Device)) <= 0)

This patch changes fixup_pmc551() by separating the error handling
and the size.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 drivers/mtd/devices/pmc551.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
index ecff765..17b3536 100644
--- a/drivers/mtd/devices/pmc551.c
+++ b/drivers/mtd/devices/pmc551.c
@@ -359,7 +359,7 @@ static int pmc551_write(struct mtd_info *mtd, loff_t to, size_t len,
  * mechanism
  * returns the size of the memory region found.
  */
-static u32 fixup_pmc551(struct pci_dev *dev)
+static int fixup_pmc551(struct pci_dev *dev, u32 *lenp)
 {
 #ifdef CONFIG_MTD_PMC551_BUGFIX
 	u32 dram_data;
@@ -638,7 +638,8 @@ static u32 fixup_pmc551(struct pci_dev *dev)
 		(bcmd & 0x1) ? "software" : "hardware",
 		(bcmd & 0x20) ? "" : "un", (bcmd & 0x40) ? "" : "un");
 #endif
-	return size;
+	*lenp = size;
+	return 0;
 }
 
 /*
@@ -713,7 +714,7 @@ static int __init init_pmc551(void)
 		 * with the oldproc.c driver in
 		 * some kernels (2.2.*)
 		 */
-		if ((length = fixup_pmc551(PCI_Device)) <= 0) {
+		if (fixup_pmc551(PCI_Device, &length) < 0 || !length) {
 			printk(KERN_NOTICE "pmc551: Cannot init SDRAM\n");
 			break;
 		}
-- 
1.7.5.4

Re: [PATCH] pmc551: fix signedness bug in init_pmc551()

[Artem Bityutskiy]

On Tue, 2011-12-27 at 15:54 -0500, Xi Wang wrote:
> Since "length" is a u32, the error handling below didn't work when
> fixup_pmc551() returns -ENODEV.
> 
> 	if ((length = fixup_pmc551(PCI_Device)) <= 0)
> 
> This patch changes fixup_pmc551() by separating the error handling
> and the size.
> 
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> ---
>  drivers/mtd/devices/pmc551.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)

You can just change the type to int without introducing a separate
argument. I think the length cannot be too large anyway. E.g., because
it is compared to "asize" which is int.

Artem.

Re: [PATCH] pmc551: fix signedness bug in init_pmc551()

[Xi Wang]

On Jan 9, 2012, at 4:05 PM, Artem Bityutskiy wrote:
> You can just change the type to int without introducing a separate
> argument. I think the length cannot be too large anyway. E.g., because
> it is compared to "asize" which is int.

That would work too.  Will do a v2.  Thanks.

- xi

Re: [PATCH] pmc551: fix signedness bug in init_pmc551()

[Xi Wang]

Since "length" is a u32, the error handling below didn't work when
fixup_pmc551() returns -ENODEV.

	if ((length = fixup_pmc551(PCI_Device)) <= 0)

This patch changes both the type of "length" and the return type of
fixup_pmc551() to int.

Suggested-by: Artem Bityutskiy <dedekind1@gmail.com>
Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 drivers/mtd/devices/pmc551.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/devices/pmc551.c b/drivers/mtd/devices/pmc551.c
index ecff765..cfccf65 100644
--- a/drivers/mtd/devices/pmc551.c
+++ b/drivers/mtd/devices/pmc551.c
@@ -359,7 +359,7 @@ static int pmc551_write(struct mtd_info *mtd, loff_t to, size_t len,
  * mechanism
  * returns the size of the memory region found.
  */
-static u32 fixup_pmc551(struct pci_dev *dev)
+static int fixup_pmc551(struct pci_dev *dev)
 {
 #ifdef CONFIG_MTD_PMC551_BUGFIX
 	u32 dram_data;
@@ -669,7 +669,7 @@ static int __init init_pmc551(void)
 	struct mypriv *priv;
 	int found = 0;
 	struct mtd_info *mtd;
-	u32 length = 0;
+	int length = 0;
 
 	if (msize) {
 		msize = (1 << (ffs(msize) - 1)) << 20;
-- 
1.7.5.4

Re: [PATCH] pmc551: fix signedness bug in init_pmc551()

[Artem Bityutskiy]

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

On Mon, 2012-01-09 at 16:58 -0500, Xi Wang wrote:
> Since "length" is a u32, the error handling below didn't work when
> fixup_pmc551() returns -ENODEV.
> 
> 	if ((length = fixup_pmc551(PCI_Device)) <= 0)
> 
> This patch changes both the type of "length" and the return type of
> fixup_pmc551() to int.

Pushed to l2-mtd.git, thanks!

-- 
Best Regards,
Artem Bityutskiy

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]