Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error

[Zhi Yong Wu]

On Thu, Aug 18, 2011 at 12:28 PM, Peter Maydell
peter.mayd...@linaro.org wrote:
 On 18 August 2011 05:16, Zhi Yong Wu zwu.ker...@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

Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error

[Zhi Yong Wu]

On Mon, Aug 1, 2011 at 8:30 AM, Anthony Liguori anth...@codemonkey.ws wrote:
 On 07/29/2011 07:18 PM, Peter Maydell wrote:

 On 29 July 2011 20:30, Stefan Weilw...@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

Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error

[Peter Maydell]

On 18 August 2011 05:16, Zhi Yong Wu zwu.ker...@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

Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error

[Anthony Liguori]

On 07/29/2011 07:18 PM, Peter Maydell wrote:

On 29 July 2011 20:30, Stefan Weilw...@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

[Qemu-devel] [PATCH] Fix gcc-4.6 compiler error

[Stefan Weil]

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 xiaw...@linux.vnet.ibm.com
Cc: Anthony Liguori aligu...@us.ibm.com
Signed-off-by: Stefan Weil w...@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))  0x;
 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))  0x;
 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

Re: [Qemu-devel] [PATCH] Fix gcc-4.6 compiler error

[Peter Maydell]

On 29 July 2011 20:30, Stefan Weil w...@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