Re: [U-Boot] [PATCH] fix: tools: kwbimage.c: Initialize headersz to suppress warning

Hi,

On 21-11-14 10:20, Lukasz Majewski wrote:

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.

Wouldn't it be better to simply disable the -Wmaybe-uninitialized for gcc?

Regards, Jeroen