linux-kernel - Re: [PATCH][MTD][NAND]: fix omap2.c compile failure and warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1246028984.663.9.camel@macbook.infradead.org>
Date:	Fri, 26 Jun 2009 16:09:44 +0100
From:	David Woodhouse <dwmw2@...radead.org>
To:	tom.leiming@...il.com, jzhang@...com, vimalsingh@...com
Cc:	linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
	david-b@...bell.net
Subject: Re: [PATCH][MTD][NAND]: fix omap2.c compile failure and warning

On Fri, 2009-06-26 at 21:21 +0800, tom.leiming@...il.com wrote:
> From: Ming Lei <tom.leiming@...il.com>
> 
> drivers/mtd/nand/omap2.c: In function 'omap_wait':
> drivers/mtd/nand/omap2.c:543: error: 'jiffies' undeclared (first use in this function)
> drivers/mtd/nand/omap2.c:543: error: (Each undeclared identifier is reported only once
> drivers/mtd/nand/omap2.c:543: error: for each function it appears in.)
> drivers/mtd/nand/omap2.c:557: error: implicit declaration of function 'time_before'

OK.

> ......
> 
> drivers/mtd/nand/omap2.c: In function 'omap_wait':
> drivers/mtd/nand/omap2.c:545: warning: 'status' may be used uninitialized in this function

Hm, why do you set it to zero? In the (fairly unlikely) case where you
never go through the loop at all, because you're preempted until the
timeout has completed, what value do you want it to return?

Don't just make changes by bashing on the keyboard until the compiler
shuts up; please apply brain.

That way, I think you'll notice that the return value from this function
is completely screwed anyway.... Vimal? Shouldn't it be returning
NAND_STATUS_FAIL in the failure case, not a value read directly from the
hardware register? And shouldn't there be a cpu_relax() (or even
schedule) in there too?

> Signed-off-by: Ming Lei <tom.leiming@...il.com>
> ---
>  drivers/mtd/nand/omap2.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/nand/omap2.c b/drivers/mtd/nand/omap2.c
> index 0cd76f8..2905c79 100644
> --- a/drivers/mtd/nand/omap2.c
> +++ b/drivers/mtd/nand/omap2.c
> @@ -15,6 +15,7 @@
>  #include <linux/mtd/nand.h>
>  #include <linux/mtd/partitions.h>
>  #include <linux/io.h>
> +#include <linux/jiffies.h>
>  
>  #include <asm/dma.h>
>  
> @@ -541,7 +542,8 @@ static int omap_wait(struct mtd_info *mtd, struct nand_chip *chip)
>  	struct omap_nand_info *info = container_of(mtd, struct omap_nand_info,
>  							mtd);
>  	unsigned long timeo = jiffies;
> -	int status, state = this->state;
> +	int state = this->state;
> +	int status = 0;
>  
>  	if (state == FL_ERASING)
>  		timeo += (HZ * 400) / 1000;
-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@...el.com                              Intel Corporation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/