From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:49178) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RLIgQ-0001QQ-37 for qemu-devel@nongnu.org; Tue, 01 Nov 2011 14:03:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RLIgK-00028u-V7 for qemu-devel@nongnu.org; Tue, 01 Nov 2011 14:03:34 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:52855) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RLIgK-00025Q-Q0 for qemu-devel@nongnu.org; Tue, 01 Nov 2011 14:03:28 -0400 Received: by mail-qy0-f180.google.com with SMTP id 38so1027217qyl.4 for ; Tue, 01 Nov 2011 11:03:28 -0700 (PDT) Message-ID: <4EB0346D.1090106@codemonkey.ws> Date: Tue, 01 Nov 2011 13:03:25 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1319455890-20667-1-git-send-email-pavel.borzenkov@gmail.com> In-Reply-To: <1319455890-20667-1-git-send-email-pavel.borzenkov@gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2] fw_cfg: Use g_file_get_contents instead of multiple fread() calls List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pavel Borzenkov Cc: qemu-devel@nongnu.org On 10/24/2011 06:31 AM, Pavel Borzenkov wrote: > Signed-off-by: Pavel Borzenkov Applied. Thanks. Regards, Anthony Liguori > --- > hw/fw_cfg.c | 102 ++++++++++++++++++++++------------------------------------- > 1 files changed, 38 insertions(+), 64 deletions(-) > > diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c > index 8df265c..dbcb888 100644 > --- a/hw/fw_cfg.c > +++ b/hw/fw_cfg.c > @@ -60,71 +60,55 @@ struct FWCfgState { > #define JPG_FILE 0 > #define BMP_FILE 1 > > -static FILE *probe_splashfile(char *filename, int *file_sizep, int *file_typep) > +static char *read_splashfile(char *filename, int *file_sizep, int *file_typep) > { > - FILE *fp = NULL; > - int fop_ret; > - int file_size; > + GError *err = NULL; > + gboolean res; > + gchar *content; > int file_type = -1; > - unsigned char buf[2] = {0, 0}; > - unsigned int filehead_value = 0; > + unsigned int filehead = 0; > int bmp_bpp; > > - fp = fopen(filename, "rb"); > - if (fp == NULL) { > - error_report("failed to open file '%s'.", filename); > - return fp; > + res = g_file_get_contents(filename,&content, (gsize *)file_sizep,&err); > + if (res == FALSE) { > + error_report("failed to read splash file '%s'", filename); > + g_error_free(err); > + return NULL; > } > + > /* check file size */ > - fseek(fp, 0L, SEEK_END); > - file_size = ftell(fp); > - if (file_size< 2) { > - error_report("file size is less than 2 bytes '%s'.", filename); > - fclose(fp); > - fp = NULL; > - return fp; > + if (*file_sizep< 30) { > + goto error; > } > + > /* 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) { > + filehead = ((content[0]& 0xff) + (content[1]<< 8))& 0xffff; > + if (filehead == 0xd8ff) { > file_type = JPG_FILE; > + } else if (filehead == 0x4d42) { > + file_type = BMP_FILE; > } else { > - if (filehead_value == 0x4d42) { > - file_type = BMP_FILE; > - } > - } > - if (file_type< 0) { > - error_report("'%s' not jpg/bmp file,head:0x%x.", > - filename, filehead_value); > - fclose(fp); > - fp = NULL; > - return fp; > + goto error; > } > + > /* check BMP bpp */ > if (file_type == BMP_FILE) { > - fseek(fp, 28, SEEK_SET); > - fop_ret = fread(buf, 1, 2, fp); > - bmp_bpp = (buf[0] + (buf[1]<< 8))& 0xffff; > + bmp_bpp = (content[28] + (content[29]<< 8))& 0xffff; > if (bmp_bpp != 24) { > - error_report("only 24bpp bmp file is supported."); > - fclose(fp); > - fp = NULL; > - return fp; > + goto error; > } > } > + > /* return values */ > - *file_sizep = file_size; > *file_typep = file_type; > - return fp; > + > + return content; > + > +error: > + error_report("splash file '%s' format not recognized; must be JPEG " > + "or 24 bit BMP", filename); > + g_free(content); > + return NULL; > } > > static void fw_cfg_bootsplash(FWCfgState *s) > @@ -132,9 +116,7 @@ static void fw_cfg_bootsplash(FWCfgState *s) > int boot_splash_time = -1; > const char *boot_splash_filename = NULL; > char *p; > - char *filename; > - FILE *fp; > - int fop_ret; > + char *filename, *file_data; > int file_size; > int file_type = -1; > const char *temp; > @@ -174,27 +156,19 @@ static void fw_cfg_bootsplash(FWCfgState *s) > error_report("failed to find file '%s'.", boot_splash_filename); > return; > } > - /* probing the file */ > - fp = probe_splashfile(filename,&file_size,&file_type); > - if (fp == NULL) { > + > + /* loading file data */ > + file_data = read_splashfile(filename,&file_size,&file_type); > + if (file_data == NULL) { > g_free(filename); > return; > } > - /* loading file data */ > if (boot_splash_filedata != NULL) { > g_free(boot_splash_filedata); > } > - boot_splash_filedata = g_malloc(file_size); > + boot_splash_filedata = (uint8_t *)file_data; > 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) { > fw_cfg_add_file(s, "bootsplash.jpg",