| Message ID | 1540357921-4199-1-git-send-email-liq3ea@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | fw_cfg: print error message when reading splashfile failed | expand |
On 10/24/18 07:12, Li Qiang wrote: > Also remove unnecessary 'res' variable. > > Signed-off-by: Li Qiang <liq3ea@gmail.com> > --- > hw/nvram/fw_cfg.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 946f765..f4a52d8 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize *file_sizep, > int *file_typep) > { > GError *err = NULL; > - gboolean res; > gchar *content; > int file_type; > unsigned int filehead; > int bmp_bpp; > > - res = g_file_get_contents(filename, &content, file_sizep, &err); > - if (res == FALSE) { > - error_report("failed to read splash file '%s'", filename); > + if (!g_file_get_contents(filename, &content, file_sizep, &err)) { > + error_report("failed to read splash file '%s', %s", > + filename, err->message); > g_error_free(err); > return NULL; > } > Seems to be OK, according to <https://developer.gnome.org/glib/stable/glib-Error-Reporting.html#glib-Error-Reporting.description>. Reviewed-by: Laszlo Ersek <lersek@redhat.com>
On 24/10/18 7:12, Li Qiang wrote: > Also remove unnecessary 'res' variable. > > Signed-off-by: Li Qiang <liq3ea@gmail.com> > --- > hw/nvram/fw_cfg.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 946f765..f4a52d8 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize *file_sizep, > int *file_typep) > { > GError *err = NULL; > - gboolean res; > gchar *content; > int file_type; > unsigned int filehead; > int bmp_bpp; > > - res = g_file_get_contents(filename, &content, file_sizep, &err); > - if (res == FALSE) { > - error_report("failed to read splash file '%s'", filename); > + if (!g_file_get_contents(filename, &content, file_sizep, &err)) { > + error_report("failed to read splash file '%s', %s", Can you use a column like the rest of the codebase? i.e.: "failed to read splash file '%s': %s" The maintainer taking this patch can do this minor change, so: Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > + filename, err->message); > g_error_free(err); > return NULL; > } >
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > On 24/10/18 7:12, Li Qiang wrote: >> Also remove unnecessary 'res' variable. >> >> Signed-off-by: Li Qiang <liq3ea@gmail.com> >> --- >> hw/nvram/fw_cfg.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c >> index 946f765..f4a52d8 100644 >> --- a/hw/nvram/fw_cfg.c >> +++ b/hw/nvram/fw_cfg.c >> @@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize *file_sizep, >> int *file_typep) >> { >> GError *err = NULL; >> - gboolean res; >> gchar *content; >> int file_type; >> unsigned int filehead; >> int bmp_bpp; >> - res = g_file_get_contents(filename, &content, file_sizep, >> &err); >> - if (res == FALSE) { >> - error_report("failed to read splash file '%s'", filename); >> + if (!g_file_get_contents(filename, &content, file_sizep, &err)) { >> + error_report("failed to read splash file '%s', %s", > > Can you use a column like the rest of the codebase? > > i.e.: "failed to read splash file '%s': %s" Yes, please. Also, my remark on your "[PATCH] vl.c: print error message if loading fw_cfg file failed" applies to this patch as well. Please improve the commit message. > The maintainer taking this patch can do this minor change, so: > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > >> + filename, err->message); >> g_error_free(err); >> return NULL; >> } >>
Hello all, I have sent out another patch. As the email's subject and commit message changed, I'm not sure is it suitable to add Philippe and Laszlo's R-b tag. Thanks, Li Qiang Markus Armbruster <armbru@redhat.com> 于2018年10月31日周三 下午11:36写道： > Philippe Mathieu-Daudé <philmd@redhat.com> writes: > > > On 24/10/18 7:12, Li Qiang wrote: > >> Also remove unnecessary 'res' variable. > >> > >> Signed-off-by: Li Qiang <liq3ea@gmail.com> > >> --- > >> hw/nvram/fw_cfg.c | 7 +++---- > >> 1 file changed, 3 insertions(+), 4 deletions(-) > >> > >> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > >> index 946f765..f4a52d8 100644 > >> --- a/hw/nvram/fw_cfg.c > >> +++ b/hw/nvram/fw_cfg.c > >> @@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize > *file_sizep, > >> int *file_typep) > >> { > >> GError *err = NULL; > >> - gboolean res; > >> gchar *content; > >> int file_type; > >> unsigned int filehead; > >> int bmp_bpp; > >> - res = g_file_get_contents(filename, &content, file_sizep, > >> &err); > >> - if (res == FALSE) { > >> - error_report("failed to read splash file '%s'", filename); > >> + if (!g_file_get_contents(filename, &content, file_sizep, &err)) { > >> + error_report("failed to read splash file '%s', %s", > > > > Can you use a column like the rest of the codebase? > > > > i.e.: "failed to read splash file '%s': %s" > > Yes, please. > > Also, my remark on your "[PATCH] vl.c: print error message if loading > fw_cfg file failed" applies to this patch as well. Please improve the > commit message. > > > The maintainer taking this patch can do this minor change, so: > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > > > >> + filename, err->message); > >> g_error_free(err); > >> return NULL; > >> } > >> >
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 946f765..f4a52d8 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize *file_sizep, int *file_typep) { GError *err = NULL; - gboolean res; gchar *content; int file_type; unsigned int filehead; int bmp_bpp; - res = g_file_get_contents(filename, &content, file_sizep, &err); - if (res == FALSE) { - error_report("failed to read splash file '%s'", filename); + if (!g_file_get_contents(filename, &content, file_sizep, &err)) { + error_report("failed to read splash file '%s', %s", + filename, err->message); g_error_free(err); return NULL; }
Also remove unnecessary 'res' variable. Signed-off-by: Li Qiang <liq3ea@gmail.com> --- hw/nvram/fw_cfg.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)