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

Jeroen Hofstee
2014-11-21T12:34:41Z

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