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

Lukasz Majewski
2014-11-21T09:20:26Z

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