| Message ID | 1312181399-29841-1-git-send-email-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
On Mon, Aug 1, 2011 at 7:49 AM, David Gibson <david@gibson.dropbear.id.au> wrote: > When compiling with gcc 4.6, some code in fw_cfg.c complains that fop_ret > is assigned but not used (which is true). However, it looks like the > meaningless assignments to fop_ret were done to suppress other gcc warnings > due to the fact that fread() is labelled as warn_unused_result in glibc. > > This patch avoids both errors, by actually checking the fread() result code > and dropping out with an error message if it fails. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- > hw/fw_cfg.c | 13 +++++++++++++ > 1 files changed, 13 insertions(+), 0 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
On Mon, Aug 01, 2011 at 11:19:38AM +0100, Stefan Hajnoczi wrote: > On Mon, Aug 1, 2011 at 7:49 AM, David Gibson > <david@gibson.dropbear.id.au> wrote: > > When compiling with gcc 4.6, some code in fw_cfg.c complains that fop_ret > > is assigned but not used (which is true). However, it looks like the > > meaningless assignments to fop_ret were done to suppress other gcc warnings > > due to the fact that fread() is labelled as warn_unused_result in glibc. > > > > This patch avoids both errors, by actually checking the fread() result code > > and dropping out with an error message if it fails. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > hw/fw_cfg.c | 13 +++++++++++++ > > 1 files changed, 13 insertions(+), 0 deletions(-) > > Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> I'm not taking this into the trivial-patches tree because a build fix like this should be merged sooner. With trivial-patches I only send one pull request per week and in the meantime people would have to ./configure --disable-werror which isn't good. Stefan
On 08/01/2011 06:19 AM, Stefan Hajnoczi wrote: > On Mon, Aug 1, 2011 at 7:49 AM, David Gibson > <david@gibson.dropbear.id.au> wrote: >> When compiling with gcc 4.6, some code in fw_cfg.c complains that fop_ret >> is assigned but not used (which is true). However, it looks like the >> meaningless assignments to fop_ret were done to suppress other gcc warnings >> due to the fact that fread() is labelled as warn_unused_result in glibc. >> >> This patch avoids both errors, by actually checking the fread() result code >> and dropping out with an error message if it fails. >> >> Signed-off-by: David Gibson<david@gibson.dropbear.id.au> >> --- >> hw/fw_cfg.c | 13 +++++++++++++ >> 1 files changed, 13 insertions(+), 0 deletions(-) > Reviewed-by: Stefan Hajnoczi<stefanha@linux.vnet.ibm.com> > Tested-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
On 08/01/2011 01:49 AM, David Gibson wrote: > When compiling with gcc 4.6, some code in fw_cfg.c complains that fop_ret > is assigned but not used (which is true). However, it looks like the > meaningless assignments to fop_ret were done to suppress other gcc warnings > due to the fact that fread() is labelled as warn_unused_result in glibc. > > This patch avoids both errors, by actually checking the fread() result code > and dropping out with an error message if it fails. > > Signed-off-by: David Gibson<david@gibson.dropbear.id.au> Applied. Thanks. Regards, Anthony Liguori > --- > hw/fw_cfg.c | 13 +++++++++++++ > 1 files changed, 13 insertions(+), 0 deletions(-) > > diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c > index a29db90..e4847b7 100644 > --- a/hw/fw_cfg.c > +++ b/hw/fw_cfg.c > @@ -87,6 +87,13 @@ 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); > + if (fop_ret != 2) { > + error_report("Could not read header from '%s': %s", > + filename, strerror(errno)); > + fclose(fp); > + fp = NULL; > + return fp; > + } > filehead_value = (buf[0] + (buf[1]<< 8))& 0xffff; > if (filehead_value == 0xd8ff) { > file_type = JPG_FILE; > @@ -181,6 +188,12 @@ static void fw_cfg_bootsplash(FWCfgState *s) > boot_splash_filedata_size = file_size; > fseek(fp, 0L, SEEK_SET); > fop_ret = fread(boot_splash_filedata, 1, file_size, fp); > + if (fop_ret != file_size) { > + error_report("failed to read data from '%s'.", > + boot_splash_filename); > + fclose(fp); > + return; > + } > fclose(fp); > /* insert data */ > if (file_type == JPG_FILE) {
diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c index a29db90..e4847b7 100644 --- a/hw/fw_cfg.c +++ b/hw/fw_cfg.c @@ -87,6 +87,13 @@ 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); + if (fop_ret != 2) { + error_report("Could not read header from '%s': %s", + filename, strerror(errno)); + fclose(fp); + fp = NULL; + return fp; + } filehead_value = (buf[0] + (buf[1] << 8)) & 0xffff; if (filehead_value == 0xd8ff) { file_type = JPG_FILE; @@ -181,6 +188,12 @@ static void fw_cfg_bootsplash(FWCfgState *s) boot_splash_filedata_size = file_size; fseek(fp, 0L, SEEK_SET); fop_ret = fread(boot_splash_filedata, 1, file_size, fp); + if (fop_ret != file_size) { + error_report("failed to read data from '%s'.", + boot_splash_filename); + fclose(fp); + return; + } fclose(fp); /* insert data */ if (file_type == JPG_FILE) {
When compiling with gcc 4.6, some code in fw_cfg.c complains that fop_ret is assigned but not used (which is true). However, it looks like the meaningless assignments to fop_ret were done to suppress other gcc warnings due to the fact that fread() is labelled as warn_unused_result in glibc. This patch avoids both errors, by actually checking the fread() result code and dropping out with an error message if it fails. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/fw_cfg.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)