From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lukasz Majewski <l.majewski@samsung.com>
Date: Fri, 21 Nov 2014 10:20:26 +0100
Subject: [U-Boot] [PATCH] fix: tools: kwbimage.c: Initialize headersz to
 suppress warning
In-Reply-To: <20141121093514.5166e9fd@free-electrons.com>
References: <1416558163-23614-1-git-send-email-l.majewski@samsung.com>
	<20141121093514.5166e9fd@free-electrons.com>
Message-ID: <20141121102026.75aad6e2@amdc2363>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Thomas,

> Dear Lukasz Majewski,
> 
> Thanks for your patch.
> 
> On Fri, 21 Nov 2014 09:22:43 +0100, Lukasz Majewski wrote:
> > When building with my toolchain (4.8.2):
> > CROSS_COMPILE=/home/lukma/work/ptxdist/toolchains/arm/OSELAS.Toolchain-2013.12.0/arm-v7a-linux-gnueabi/gcc-4.8.2-glibc-2.18-binutils-2.24-kernel-3.12-sanitized/bin/arm-v7a-linux-gnueabi-
> 
> Well, your target toolchain doesn't have much to do about the issue.
> tools/kwbimage.c is built for the host.

Yes. Correct.

Host: gcc version 4.7.2 (Debian 4.7.2-5)

> 
> > I see following WARNING:
> > tools/kwbimage.c: In function "kwbimage_set_header":
> > tools/kwbimage.c:803:8: warning: "headersz" may be used
> > uninitialized in this function [-Wmaybe-uninitialized] memcpy(ptr,
> > image, headersz); ^
> > This fix aims to suppress it.
> > 
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > ---
> >  tools/kwbimage.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/kwbimage.c b/tools/kwbimage.c
> > index c50f2e2..2c302e5 100644
> > --- a/tools/kwbimage.c
> > +++ b/tools/kwbimage.c
> > @@ -728,7 +728,7 @@ static void kwbimage_set_header(void *ptr,
> > struct stat *sbuf, int ifd, FILE *fcfg;
> >  	void *image = NULL;
> >  	int version;
> > -	size_t headersz;
> > +	size_t headersz = 0;
> >  	uint32_t checksum;
> >  	int ret;
> >  	int size;
> 
> Looking briefly again at the code, I believe the warning from gcc is
> probably bogus. Here is the code:
> 
>         size_t headersz;
> 	[...]
>         version = image_get_version();
>         switch (version) {
>                 /*
>                  * Fallback to version 0 if no version is provided in
> the
>                  * cfg file
>                  */
>         case -1:
>         case 0:
>                 image = image_create_v0(&headersz, params,
> sbuf->st_size); break;
> 
>         case 1:
>                 image = image_create_v1(&headersz, params,
> sbuf->st_size); break;
> 
>         default:
>                 fprintf(stderr, "Unsupported version %d\n", version);
>                 free(image_cfg);
>                 exit(EXIT_FAILURE);
>         }
> 	[...]
>         /* Finally copy the header into the image area */
>         memcpy(ptr, image, headersz);
> 
> So the usage of 'headersz' is only done if we have gone through either
> the -1/0/1 cases. In the 'default' case, we exit the tool, so the
> memcpy() is never reached. Maybe gcc doesn't realize we're getting out
> of the function in the default case.
> 
> But oh well, if it fixes a warning :-)

I didn't claim that there is a bug in the code :-).

I just get annoying when on my continuous integration script I see the
same warning for all cross compiled boards.

Anyway, I assume that you don't have any objections to this patch :-) .

> 
> Best regards,
> 
> Thomas


-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group