[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] fw_cfg: check return value of fread()
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] fw_cfg: check return value of fread() |
Date: |
Tue, 18 Oct 2011 18:30:54 +0100 |
On 18 October 2011 18:16, Pavel Borzenkov <address@hidden> wrote:
> Spotted by Clang Analyzer
>
> Signed-off-by: Pavel Borzenkov <address@hidden>
> ---
> hw/fw_cfg.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
> index 8df265c..1125e7d 100644
> --- a/hw/fw_cfg.c
> +++ b/hw/fw_cfg.c
> @@ -113,6 +113,13 @@ static FILE *probe_splashfile(char *filename, int
> *file_sizep, int *file_typep)
> if (file_type == BMP_FILE) {
> fseek(fp, 28, SEEK_SET);
> fop_ret = fread(buf, 1, 2, fp);
> + if (fop_ret != 2) {
> + error_report("Could not read bpp value from '%s': %s",
> + filename, strerror(errno));
> + fclose(fp);
> + fp = NULL;
> + return fp;
> + }
> bmp_bpp = (buf[0] + (buf[1] << 8)) & 0xffff;
> if (bmp_bpp != 24) {
> error_report("only 24bpp bmp file is supported.");
Yuck, this code again. We should just replace it with
g_file_get_contents() and looking at the resulting buffer.
That would be 10% of the code and much less bug-ridden.
FWIW, strictly speaking there isn't a need to check the result
of the fread() because if we don't read the data then buf[] will
still have the BMP_FILE signature in it and the != 24 check will
fail.
Not checking the return code from that fseek(), on the other hand...
-- PMM