diff mbox

[v2] fw_cfg: Use g_file_get_contents instead of multiple fread() calls

Message ID 1319455890-20667-1-git-send-email-pavel.borzenkov@gmail.com
State New
Headers show

Commit Message

Pavel Borzenkov Oct. 24, 2011, 11:31 a.m. UTC 
Signed-off-by: Pavel Borzenkov <pavel.borzenkov@gmail.com>
---
 hw/fw_cfg.c |  102 ++++++++++++++++++++++-------------------------------------
 1 files changed, 38 insertions(+), 64 deletions(-)

Comments

Pavel Borzenkov Oct. 27, 2011, 10:12 a.m. UTC | #1 
Ping?

On Mon, Oct 24, 2011 at 3:31 PM, Pavel Borzenkov
<pavel.borzenkov@gmail.com> wrote:
> Signed-off-by: Pavel Borzenkov <pavel.borzenkov@gmail.com>
> ---
>  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",
> --
> 1.7.0.4
>
>
>
Peter Maydell Oct. 27, 2011, 11:49 a.m. UTC | #2 
On 24 October 2011 12:31, Pavel Borzenkov <pavel.borzenkov@gmail.com> wrote:
> Signed-off-by: Pavel Borzenkov <pavel.borzenkov@gmail.com>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

-- PMM
Anthony Liguori Nov. 1, 2011, 6:03 p.m. UTC | #3 
On 10/24/2011 06:31 AM, Pavel Borzenkov wrote:
> Signed-off-by: Pavel Borzenkov<pavel.borzenkov@gmail.com>

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",
diff mbox

Patch

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",