From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jeroen Hofstee <jeroen@myspectrum.nl>
Date: Fri, 21 Nov 2014 13:34:41 +0100
Subject: [U-Boot] [PATCH] fix: tools: kwbimage.c: Initialize headersz to
 suppress warning
In-Reply-To: <20141121102026.75aad6e2@amdc2363>
References: <1416558163-23614-1-git-send-email-l.majewski@samsung.com>	<20141121093514.5166e9fd@free-electrons.com>
	<20141121102026.75aad6e2@amdc2363>
Message-ID: <546F3161.1040906@myspectrum.nl>
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,

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