| Message ID | 1420663040-3857-1-git-send-email-andy.shevchenko@gmail.com |
|---|---|
| State | Accepted |
| Commit | 768c57c8d7df479aa27330d629b4e41f9c19b22c |
| Headers | show |
On Wed, Jan 07, 2015 at 10:37:20PM +0200, Andy Shevchenko wrote: > We need to compare ret variable for negative value. The current code > assigns the boolean to the ret and prints it wrongly in the warning > message. > > Reported-by: Andrey Karpov <karpov@viva64.com> > Cc: Giel van Schijndel <me@mortis.eu> > Cc: Dimitri Gorokhovik <dimitri.gorokhovik@free.fr> > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> To be clear, this touches some commented out code (yuck). I think you noted this previously. For my reference, are you actually testing this driver? Anyway, pushed to l2-mtd.git. Thanks. Brian
On Sat, Jan 10, 2015 at 1:29 AM, Brian Norris <computersforpeace@gmail.com> wrote: > On Wed, Jan 07, 2015 at 10:37:20PM +0200, Andy Shevchenko wrote: >> We need to compare ret variable for negative value. The current code >> assigns the boolean to the ret and prints it wrongly in the warning >> message. >> >> Reported-by: Andrey Karpov <karpov@viva64.com> >> Cc: Giel van Schijndel <me@mortis.eu> >> Cc: Dimitri Gorokhovik <dimitri.gorokhovik@free.fr> >> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > To be clear, this touches some commented out code (yuck). I think you > noted this previously. It had been proposed by Giel. > > For my reference, are you actually testing this driver? > Not a real testing. Only compilation on x86_32. > Anyway, pushed to l2-mtd.git. Thanks. Thanks! > > Brian
On Sat, Jan 10, 2015 at 14:56:32 +0200, Andy Shevchenko wrote: > On Sat, Jan 10, 2015 at 1:29 AM, Brian Norris <computersforpeace@gmail.com> wrote: >> On Wed, Jan 07, 2015 at 10:37:20PM +0200, Andy Shevchenko wrote: >>> We need to compare ret variable for negative value. The current code >>> assigns the boolean to the ret and prints it wrongly in the warning >>> message. >>> >>> Reported-by: Andrey Karpov <karpov@viva64.com> >>> Cc: Giel van Schijndel <me@mortis.eu> >>> Cc: Dimitri Gorokhovik <dimitri.gorokhovik@free.fr> >>> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> >> >> To be clear, this touches some commented out code (yuck). I think you >> noted this previously. > > It had been proposed by Giel. Yes, and I suspect (and hope) Brian's referring to the existence of commented-out code as "yuck", not the removal of bugs from it. >> For my reference, are you actually testing this driver? > > Not a real testing. Only compilation on x86_32. Combined with review I think that's enough given the nature of this change.
On Sat, Jan 10, 2015 at 09:11:13PM +0100, Giel van Schijndel wrote: > On Sat, Jan 10, 2015 at 14:56:32 +0200, Andy Shevchenko wrote: > > On Sat, Jan 10, 2015 at 1:29 AM, Brian Norris <computersforpeace@gmail.com> wrote: > >> On Wed, Jan 07, 2015 at 10:37:20PM +0200, Andy Shevchenko wrote: > >>> We need to compare ret variable for negative value. The current code > >>> assigns the boolean to the ret and prints it wrongly in the warning > >>> message. > >>> > >>> Reported-by: Andrey Karpov <karpov@viva64.com> > >>> Cc: Giel van Schijndel <me@mortis.eu> > >>> Cc: Dimitri Gorokhovik <dimitri.gorokhovik@free.fr> > >>> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> > >> > >> To be clear, this touches some commented out code (yuck). I think you > >> noted this previously. > > > > It had been proposed by Giel. > > Yes, and I suspect (and hope) Brian's referring to the existence of > commented-out code as "yuck", not the removal of bugs from it. Yes, I was just commenting about the existence of all that dead code. The code won't even compile now, as it's using some old interfaces. > >> For my reference, are you actually testing this driver? > > > > Not a real testing. Only compilation on x86_32. > > Combined with review I think that's enough given the nature of this > change. Yeah, I wasn't objecting to fixing this trivial stuff. I just don't know who uses some of the aging drivers we have in MTD. Brian
diff --git a/drivers/mtd/nftlmount.c b/drivers/mtd/nftlmount.c index 51b9d6a..a5dfbfb 100644 --- a/drivers/mtd/nftlmount.c +++ b/drivers/mtd/nftlmount.c @@ -89,9 +89,10 @@ static int find_boot_record(struct NFTLrecord *nftl) } /* To be safer with BIOS, also use erase mark as discriminant */ - if ((ret = nftl_read_oob(mtd, block * nftl->EraseSize + + ret = nftl_read_oob(mtd, block * nftl->EraseSize + SECTORSIZE + 8, 8, &retlen, - (char *)&h1) < 0)) { + (char *)&h1); + if (ret < 0) { printk(KERN_WARNING "ANAND header found at 0x%x in mtd%d, but OOB data read failed (err %d)\n", block * nftl->EraseSize, nftl->mbd.mtd->index, ret); continue; @@ -109,8 +110,9 @@ static int find_boot_record(struct NFTLrecord *nftl) } /* Finally reread to check ECC */ - if ((ret = mtd->read(mtd, block * nftl->EraseSize, SECTORSIZE, - &retlen, buf) < 0)) { + ret = mtd->read(mtd, block * nftl->EraseSize, SECTORSIZE, + &retlen, buf); + if (ret < 0) { printk(KERN_NOTICE "ANAND header found at 0x%x in mtd%d, but ECC read failed (err %d)\n", block * nftl->EraseSize, nftl->mbd.mtd->index, ret); continue; @@ -228,9 +230,11 @@ device is already correct. The new DiskOnChip driver already scanned the bad block table. Just query it. if ((i & (SECTORSIZE - 1)) == 0) { /* read one sector for every SECTORSIZE of blocks */ - if ((ret = mtd->read(nftl->mbd.mtd, block * nftl->EraseSize + - i + SECTORSIZE, SECTORSIZE, &retlen, - buf)) < 0) { + ret = mtd->read(nftl->mbd.mtd, + block * nftl->EraseSize + i + + SECTORSIZE, SECTORSIZE, + &retlen, buf); + if (ret < 0) { printk(KERN_NOTICE "Read of bad sector table failed (err %d)\n", ret); kfree(nftl->ReplUnitTable);
We need to compare ret variable for negative value. The current code assigns the boolean to the ret and prints it wrongly in the warning message. Reported-by: Andrey Karpov <karpov@viva64.com> Cc: Giel van Schijndel <me@mortis.eu> Cc: Dimitri Gorokhovik <dimitri.gorokhovik@free.fr> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> --- drivers/mtd/nftlmount.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)