* [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error
@ 2011-07-29 19:30 Stefan Weil
2011-07-30 0:18 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Weil @ 2011-07-29 19:30 UTC (permalink / raw)
To: QEMU Developers; +Cc: Anthony Liguori, Wayne Xia
Commit 3d3b8303c6f83b9b245bc774af530a6403cc4ce6
breaks builds with gcc-4.6:
hw/fw_cfg.c: In function ‘probe_splashfile’:
hw/fw_cfg.c:66:9: error: variable ‘fop_ret’ set but not used [-Werror=unused-but-set-variable]
hw/fw_cfg.c: In function ‘fw_cfg_bootsplash’:
hw/fw_cfg.c:130:9: error: variable ‘fop_ret’ set but not used [-Werror=unused-but-set-variable]
Remove fop_ret. Testing the result of fread() is normally
a good idea, but I don't think it is needed here.
Cc: Wayne Xia <xiawenc@linux.vnet.ibm.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
hw/fw_cfg.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index a29db90..d906b83 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -63,7 +63,6 @@ struct FWCfgState {
static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep)
{
FILE *fp = NULL;
- int fop_ret;
int file_size;
int file_type = -1;
unsigned char buf[2] = {0, 0};
@@ -86,7 +85,7 @@ static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep)
}
/* check magic ID */
fseek(fp, 0L, SEEK_SET);
- fop_ret = fread(buf, 1, 2, fp);
+ (void)fread(buf, 1, 2, fp);
filehead_value = (buf[0] + (buf[1] << 8)) & 0xffff;
if (filehead_value == 0xd8ff) {
file_type = JPG_FILE;
@@ -105,7 +104,7 @@ static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep)
/* check BMP bpp */
if (file_type == BMP_FILE) {
fseek(fp, 28, SEEK_SET);
- fop_ret = fread(buf, 1, 2, fp);
+ (void)fread(buf, 1, 2, fp);
bmp_bpp = (buf[0] + (buf[1] << 8)) & 0xffff;
if (bmp_bpp != 24) {
error_report("only 24bpp bmp file is supported.");
@@ -127,7 +126,6 @@ static void fw_cfg_bootsplash(FWCfgState *s)
char *p;
char *filename;
FILE *fp;
- int fop_ret;
int file_size;
int file_type = -1;
const char *temp;
@@ -180,7 +178,7 @@ static void fw_cfg_bootsplash(FWCfgState *s)
boot_splash_filedata = qemu_malloc(file_size);
boot_splash_filedata_size = file_size;
fseek(fp, 0L, SEEK_SET);
- fop_ret = fread(boot_splash_filedata, 1, file_size, fp);
+ (void)fread(boot_splash_filedata, 1, file_size, fp);
fclose(fp);
/* insert data */
if (file_type == JPG_FILE) {
--
1.7.2.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error
2011-07-29 19:30 [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error Stefan Weil
@ 2011-07-30 0:18 ` Peter Maydell
2011-08-01 0:30 ` Anthony Liguori
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2011-07-30 0:18 UTC (permalink / raw)
To: Stefan Weil; +Cc: Anthony Liguori, QEMU Developers, Wayne Xia
On 29 July 2011 20:30, Stefan Weil <weil@mail.berlios.de> wrote:
> Commit 3d3b8303c6f83b9b245bc774af530a6403cc4ce6
> breaks builds with gcc-4.6:
>
> hw/fw_cfg.c: In function ‘probe_splashfile’:
> hw/fw_cfg.c:66:9: error: variable ‘fop_ret’ set but not used [-Werror=unused-but-set-variable]
> hw/fw_cfg.c: In function ‘fw_cfg_bootsplash’:
> hw/fw_cfg.c:130:9: error: variable ‘fop_ret’ set but not used [-Werror=unused-but-set-variable]
>
> Remove fop_ret. Testing the result of fread() is normally
> a good idea, but I don't think it is needed here.
> @@ -86,7 +85,7 @@ static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep)
> }
> /* check magic ID */
> fseek(fp, 0L, SEEK_SET);
> - fop_ret = fread(buf, 1, 2, fp);
> + (void)fread(buf, 1, 2, fp);
Usually this kind of thing is added in order to stop gcc complaining about
you ignoring the return value of a function which has been marked (by libc)
as 'don't-ignore-return-value'. In such cases a "(void)" is not sufficient
to suppress the "return value ignored" warning.
At least some of these cases really should be checking fread return values;
I see we also don't check fseek() return values either in all places. So
the easiest fix is just to check all the fread() calls.
Alternative suggestion: it would be easier to just slurp the whole file
into memory (which is what we do once we've figured out it's an image)
and then check the magic numbers in the memory buffer, which removes
a lot of these unchecked function calls altogether. Since we're linking
against glib anyway it looks like g_file_get_contents() would do 95%
of the work for us. [disclaimer: I haven't used that API myself but it
looks the right shape...g_malloc vs qemu_malloc issues, maybe?] Failing
that, fopen/fstat/fread/fclose/check magic numbers.
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error
2011-07-30 0:18 ` Peter Maydell
@ 2011-08-01 0:30 ` Anthony Liguori
2011-08-18 4:16 ` Zhi Yong Wu
0 siblings, 1 reply; 6+ messages in thread
From: Anthony Liguori @ 2011-08-01 0:30 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, Wayne Xia
On 07/29/2011 07:18 PM, Peter Maydell wrote:
> On 29 July 2011 20:30, Stefan Weil<weil@mail.berlios.de> wrote:
>> Commit 3d3b8303c6f83b9b245bc774af530a6403cc4ce6
>> breaks builds with gcc-4.6:
>>
>> hw/fw_cfg.c: In function ‘probe_splashfile’:
>> hw/fw_cfg.c:66:9: error: variable ‘fop_ret’ set but not used [-Werror=unused-but-set-variable]
>> hw/fw_cfg.c: In function ‘fw_cfg_bootsplash’:
>> hw/fw_cfg.c:130:9: error: variable ‘fop_ret’ set but not used [-Werror=unused-but-set-variable]
>>
>> Remove fop_ret. Testing the result of fread() is normally
>> a good idea, but I don't think it is needed here.
>
>> @@ -86,7 +85,7 @@ static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep)
>> }
>> /* check magic ID */
>> fseek(fp, 0L, SEEK_SET);
>> - fop_ret = fread(buf, 1, 2, fp);
>> + (void)fread(buf, 1, 2, fp);
>
> Usually this kind of thing is added in order to stop gcc complaining about
> you ignoring the return value of a function which has been marked (by libc)
> as 'don't-ignore-return-value'. In such cases a "(void)" is not sufficient
> to suppress the "return value ignored" warning.
>
> At least some of these cases really should be checking fread return values;
> I see we also don't check fseek() return values either in all places. So
> the easiest fix is just to check all the fread() calls.
>
> Alternative suggestion: it would be easier to just slurp the whole file
> into memory (which is what we do once we've figured out it's an image)
> and then check the magic numbers in the memory buffer, which removes
> a lot of these unchecked function calls altogether. Since we're linking
> against glib anyway it looks like g_file_get_contents() would do 95%
> of the work for us. [disclaimer: I haven't used that API myself but it
> looks the right shape...g_malloc vs qemu_malloc issues, maybe?] Failing
> that, fopen/fstat/fread/fclose/check magic numbers.
As long as it's not mixed, it shouldn't be a problem.
I think using g_file_get_contents would make sense here.
Regards,
Anthony Liguori
>
> -- PMM
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error
2011-08-01 0:30 ` Anthony Liguori
@ 2011-08-18 4:16 ` Zhi Yong Wu
2011-08-18 4:28 ` Peter Maydell
0 siblings, 1 reply; 6+ messages in thread
From: Zhi Yong Wu @ 2011-08-18 4:16 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Peter Maydell, QEMU Developers
On Mon, Aug 1, 2011 at 8:30 AM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 07/29/2011 07:18 PM, Peter Maydell wrote:
>>
>> On 29 July 2011 20:30, Stefan Weil<weil@mail.berlios.de> wrote:
>>>
>>> Commit 3d3b8303c6f83b9b245bc774af530a6403cc4ce6
>>> breaks builds with gcc-4.6:
>>>
>>> hw/fw_cfg.c: In function ‘probe_splashfile’:
>>> hw/fw_cfg.c:66:9: error: variable ‘fop_ret’ set but not used
>>> [-Werror=unused-but-set-variable]
>>> hw/fw_cfg.c: In function ‘fw_cfg_bootsplash’:
>>> hw/fw_cfg.c:130:9: error: variable ‘fop_ret’ set but not used
>>> [-Werror=unused-but-set-variable]
>>>
>>> Remove fop_ret. Testing the result of fread() is normally
>>> a good idea, but I don't think it is needed here.
>>
>>> @@ -86,7 +85,7 @@ static FILE *probe_splashfile(char *filename, int
>>> *file_sizep, int *file_typep)
>>> }
>>> /* check magic ID */
>>> fseek(fp, 0L, SEEK_SET);
>>> - fop_ret = fread(buf, 1, 2, fp);
>>> + (void)fread(buf, 1, 2, fp);
>>
>> Usually this kind of thing is added in order to stop gcc complaining about
>> you ignoring the return value of a function which has been marked (by
>> libc)
>> as 'don't-ignore-return-value'. In such cases a "(void)" is not sufficient
>> to suppress the "return value ignored" warning.
>>
>> At least some of these cases really should be checking fread return
>> values;
>> I see we also don't check fseek() return values either in all places. So
>> the easiest fix is just to check all the fread() calls.
>>
>> Alternative suggestion: it would be easier to just slurp the whole file
>> into memory (which is what we do once we've figured out it's an image)
>> and then check the magic numbers in the memory buffer, which removes
>> a lot of these unchecked function calls altogether. Since we're linking
>> against glib anyway it looks like g_file_get_contents() would do 95%
>> of the work for us. [disclaimer: I haven't used that API myself but it
>> looks the right shape...g_malloc vs qemu_malloc issues, maybe?] Failing
>> that, fopen/fstat/fread/fclose/check magic numbers.
>
> As long as it's not mixed, it shouldn't be a problem.
>
> I think using g_file_get_contents would make sense here.
I have also met this problem on fedora 15 today. Currently i disable
werror option to build successfully. How to completely this problem
regardless of gcc>=4.6?
>
> Regards,
>
> Anthony Liguori
>
>>
>> -- PMM
>>
>>
>
>
>
--
Regards,
Zhi Yong Wu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error
2011-08-18 4:16 ` Zhi Yong Wu
@ 2011-08-18 4:28 ` Peter Maydell
2011-08-19 2:48 ` Zhi Yong Wu
0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2011-08-18 4:28 UTC (permalink / raw)
To: Zhi Yong Wu; +Cc: QEMU Developers
On 18 August 2011 05:16, Zhi Yong Wu <zwu.kernel@gmail.com> wrote:
> I have also met this problem on fedora 15 today. Currently i disable
> werror option to build successfully. How to completely this problem
> regardless of gcc>=4.6?
Hmm, this should have been fixed by commit 257a73755. Can you
tell us which git revision you are trying to build and quote
the complete error message from gcc, please?
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error
2011-08-18 4:28 ` Peter Maydell
@ 2011-08-19 2:48 ` Zhi Yong Wu
0 siblings, 0 replies; 6+ messages in thread
From: Zhi Yong Wu @ 2011-08-19 2:48 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers
On Thu, Aug 18, 2011 at 12:28 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 18 August 2011 05:16, Zhi Yong Wu <zwu.kernel@gmail.com> wrote:
>> I have also met this problem on fedora 15 today. Currently i disable
>> werror option to build successfully. How to completely this problem
>> regardless of gcc>=4.6?
>
> Hmm, this should have been fixed by commit 257a73755. Can you
> tell us which git revision you are trying to build and quote
> the complete error message from gcc, please?
Sorry, my branch is a bit old. After rebasing it to latest version,
the fix has checked in and this issue has been resolved. thanks.
>
> thanks
> -- PMM
>
--
Regards,
Zhi Yong Wu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-08-19 2:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-29 19:30 [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error Stefan Weil
2011-07-30 0:18 ` Peter Maydell
2011-08-01 0:30 ` Anthony Liguori
2011-08-18 4:16 ` Zhi Yong Wu
2011-08-18 4:28 ` Peter Maydell
2011-08-19 2:48 ` Zhi Yong Wu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).