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

[Jeroen Hofstee]

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 at 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