Message ID | 1449743372-17169-7-git-send-email-armbru@redhat.com |
---|---|
State | New |
Headers | show |
On 10 December 2015 at 10:29, Markus Armbruster <armbru@redhat.com> wrote: > virt_set_gic_version() calls exit(1) when passed an invalid property > value. Property setters are not supposed to do that. Screwed up in > commit b92ad39. Harmless, because the property belongs to a machine. > Set an error object instead. > > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: qemu-arm@nongnu.org > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > hw/arm/virt.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 9c6792c..2a120dd 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -1126,9 +1126,8 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp) > } else if (!strcmp(value, "host")) { > vms->gic_version = 0; /* Will probe later */ > } else { > - error_report("Invalid gic-version option value"); > - error_printf("Allowed gic-version values are: 3, 2, host\n"); > - exit(1); > + error_setg(errp, "Invalid gic-version value"); > + error_append_hint(errp, "Valid values are: 3, 2, host\n"); > } Should hint strings have trailing newlines? Otherwise Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
Peter Maydell <peter.maydell@linaro.org> writes: > On 10 December 2015 at 10:29, Markus Armbruster <armbru@redhat.com> wrote: >> virt_set_gic_version() calls exit(1) when passed an invalid property >> value. Property setters are not supposed to do that. Screwed up in >> commit b92ad39. Harmless, because the property belongs to a machine. >> Set an error object instead. >> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: qemu-arm@nongnu.org >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> hw/arm/virt.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 9c6792c..2a120dd 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -1126,9 +1126,8 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp) >> } else if (!strcmp(value, "host")) { >> vms->gic_version = 0; /* Will probe later */ >> } else { >> - error_report("Invalid gic-version option value"); >> - error_printf("Allowed gic-version values are: 3, 2, host\n"); >> - exit(1); >> + error_setg(errp, "Invalid gic-version value"); >> + error_append_hint(errp, "Valid values are: 3, 2, host\n"); >> } > > Should hint strings have trailing newlines? Two answers: 1. No, because error_report_err() prints the hint followed by a newline. 2. Yes, because error_appent_hint() accumulates, and requiring its users to know which call will be the final one is an awkward interface. I'll change error_report_err() and improve the documentation. > Otherwise > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Thanks!
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 9c6792c..2a120dd 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1126,9 +1126,8 @@ static void virt_set_gic_version(Object *obj, const char *value, Error **errp) } else if (!strcmp(value, "host")) { vms->gic_version = 0; /* Will probe later */ } else { - error_report("Invalid gic-version option value"); - error_printf("Allowed gic-version values are: 3, 2, host\n"); - exit(1); + error_setg(errp, "Invalid gic-version value"); + error_append_hint(errp, "Valid values are: 3, 2, host\n"); } }
virt_set_gic_version() calls exit(1) when passed an invalid property value. Property setters are not supposed to do that. Screwed up in commit b92ad39. Harmless, because the property belongs to a machine. Set an error object instead. Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/arm/virt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)