Re: [Qemu-devel] [PATCH] fw_cfg: Use g_file_get_contents instead of multiple fread() calls

[Peter Maydell]

On 21 October 2011 12:35, Pavel Borzenkov <address@hidden> wrote:
> Signed-off-by: Pavel Borzenkov <address@hidden>

Thanks; I think this looks a lot nicer than the raw file ops code did.
I think we could probably make the error messages slightly more user
friendly while we're here.

> +    res = g_file_get_contents(filename, &content, (gsize *)file_sizep, &err);
> +    if (res == FALSE) {
> +        error_report("falied to read file '%s'", filename);

You've made a typo here ("failed"); also I think all the error messages
in this function should say "splash file" rather than just "file",
and include the filename.

> +    if (*file_sizep < 30) {
> +        error_report("file size is less than 30 bytes '%s'", filename);
> +        g_free(content);
> +        return NULL;

Rather than saying exactly what check failed (the user won't care)
we should just say "splash file '%s' format not recognized; must be JPEG
or 24 bit BMP".

> +        error_report("'%s' not jpg/bmp file, head:0x%x.", filename, 
> filehead);

and here also.

>         if (bmp_bpp != 24) {
>             error_report("only 24bpp bmp file is supported.");

and here I think.

-- PMM