| Message ID | 624f54b5c269178e6fb4b212de05ceda59c3d009.1364487348.git.phrdina@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/28/2013 10:47 AM, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina@redhat.com> > --- > qemu-img.c | 18 ++++++++++++------ > 1 file changed, 12 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com> > +static int qemu_img_handle_error(Error *err) > +{ > + if (error_is_set(&err)) { > + error_report("%s", error_get_pretty(err)); > + error_free(err); > + return 1; A non-bool positive number is unusual for errors (-1, or a true/false scheme, tend to be more common)... > - if (error_is_set(&local_err)) { > - error_report("%s", error_get_pretty(local_err)); > - error_free(local_err); > - return 1; ...but it is accurate code motion, so it doesn't impact my review.
On 03/28/2013 03:42 PM, Eric Blake wrote: > On 03/28/2013 10:47 AM, Pavel Hrdina wrote: >> Signed-off-by: Pavel Hrdina <phrdina@redhat.com> >> --- >> qemu-img.c | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> > >> +static int qemu_img_handle_error(Error *err) >> +{ >> + if (error_is_set(&err)) { >> + error_report("%s", error_get_pretty(err)); >> + error_free(err); >> + return 1; > > A non-bool positive number is unusual for errors (-1, or a true/false > scheme, tend to be more common)... > >> - if (error_is_set(&local_err)) { >> - error_report("%s", error_get_pretty(local_err)); >> - error_free(local_err); >> - return 1; > > ...but it is accurate code motion, so it doesn't impact my review. And in reading 9/11, I found out why it struck me as unusual. I usually spell it EXIT_FAILURE instead of 1, for a return status intended to be used in a final exit() or return from main(). But that's a pre-existing syndrome that affects much more of qemu-img.c, not worth changing here.
diff --git a/qemu-img.c b/qemu-img.c index 21d02bf..d5f81cc 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -322,6 +322,17 @@ static int add_old_style_options(const char *fmt, QEMUOptionParameter *list, return 0; } +static int qemu_img_handle_error(Error *err) +{ + if (error_is_set(&err)) { + error_report("%s", error_get_pretty(err)); + error_free(err); + return 1; + } + + return 0; +} + static int img_create(int argc, char **argv) { int c; @@ -400,13 +411,8 @@ static int img_create(int argc, char **argv) bdrv_img_create(filename, fmt, base_filename, base_fmt, options, img_size, BDRV_O_FLAGS, &local_err, quiet); - if (error_is_set(&local_err)) { - error_report("%s", error_get_pretty(local_err)); - error_free(local_err); - return 1; - } - return 0; + return qemu_img_handle_error(local_err); } static void dump_json_image_check(ImageCheck *check, bool quiet)
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- qemu-img.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)