| Message ID | 20211029230147.2465055-5-philmd@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/core: Remove uses of obsolete QERR_ definitions | expand |
On 10/30/21 01:01, Philippe Mathieu-Daudé wrote: > QERR_PROPERTY_VALUE_BAD definition is obsolete since 2015 (commit > 4629ed1e989, "qerror: Finally unused, clean up"). Replace the two > uses and drop the definition. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/qapi/qmp/qerror.h | 3 --- > hw/core/qdev-properties.c | 2 +- > target/i386/cpu.c | 2 +- > 3 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h > index f49ae01cdb0..a3f44fc4a1e 100644 > --- a/include/qapi/qmp/qerror.h > +++ b/include/qapi/qmp/qerror.h > @@ -50,9 +50,6 @@ > #define QERR_PERMISSION_DENIED \ > "Insufficient permission to perform this operation" > > -#define QERR_PROPERTY_VALUE_BAD \ > - "Property '%s.%s' doesn't take value '%s'" > - > #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ > "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index c34aac6ebc9..dbea4cf8e5e 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -663,7 +663,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj, > break; > default: > case -EINVAL: > - error_setg(errp, QERR_PROPERTY_VALUE_BAD, > + error_setg(errp, "Property '%s.%s' doesn't take value '%s'", > object_get_typename(obj), name, value); > break; > case -ENOENT: > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index fc3ed80ef1e..bc63b80e5bd 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -4469,7 +4469,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > int i; > > if (strlen(value) != CPUID_VENDOR_SZ) { > - error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value); > + error_setg(errp, "Property '.vendor' doesn't take value '%s'", value); > return; > } > > Hi, maybe we can remove the '.' before vendor in this case. -- Damien
On 11/2/21 10:47, Damien Hedde wrote: > On 10/30/21 01:01, Philippe Mathieu-Daudé wrote: >> QERR_PROPERTY_VALUE_BAD definition is obsolete since 2015 (commit >> 4629ed1e989, "qerror: Finally unused, clean up"). Replace the two >> uses and drop the definition. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> include/qapi/qmp/qerror.h | 3 --- >> hw/core/qdev-properties.c | 2 +- >> target/i386/cpu.c | 2 +- >> 3 files changed, 2 insertions(+), 5 deletions(-) >> >> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h >> index f49ae01cdb0..a3f44fc4a1e 100644 >> --- a/include/qapi/qmp/qerror.h >> +++ b/include/qapi/qmp/qerror.h >> @@ -50,9 +50,6 @@ >> #define QERR_PERMISSION_DENIED \ >> "Insufficient permission to perform this operation" >> -#define QERR_PROPERTY_VALUE_BAD \ >> - "Property '%s.%s' doesn't take value '%s'" >> - >> #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ >> "Property %s.%s doesn't take value %" PRId64 " (minimum: %" >> PRId64 ", maximum: %" PRId64 ")" >> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c >> index c34aac6ebc9..dbea4cf8e5e 100644 >> --- a/hw/core/qdev-properties.c >> +++ b/hw/core/qdev-properties.c >> @@ -663,7 +663,7 @@ void error_set_from_qdev_prop_error(Error **errp, >> int ret, Object *obj, >> break; >> default: >> case -EINVAL: >> - error_setg(errp, QERR_PROPERTY_VALUE_BAD, >> + error_setg(errp, "Property '%s.%s' doesn't take value '%s'", >> object_get_typename(obj), name, value); >> break; >> case -ENOENT: >> diff --git a/target/i386/cpu.c b/target/i386/cpu.c >> index fc3ed80ef1e..bc63b80e5bd 100644 >> --- a/target/i386/cpu.c >> +++ b/target/i386/cpu.c >> @@ -4469,7 +4469,7 @@ static void x86_cpuid_set_vendor(Object *obj, >> const char *value, >> int i; >> if (strlen(value) != CPUID_VENDOR_SZ) { >> - error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value); >> + error_setg(errp, "Property '.vendor' doesn't take value >> '%s'", value); >> return; >> } >> > Hi, > > maybe we can remove the '.' before vendor in this case. I think so too but have no clue about this are, so will let the x86 maintainers decide (I have to respin anyway).
Philippe Mathieu-Daudé <philmd@redhat.com> writes: > QERR_PROPERTY_VALUE_BAD definition is obsolete since 2015 (commit > 4629ed1e989, "qerror: Finally unused, clean up"). Replace the two > uses and drop the definition. > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > include/qapi/qmp/qerror.h | 3 --- > hw/core/qdev-properties.c | 2 +- > target/i386/cpu.c | 2 +- > 3 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h > index f49ae01cdb0..a3f44fc4a1e 100644 > --- a/include/qapi/qmp/qerror.h > +++ b/include/qapi/qmp/qerror.h > @@ -50,9 +50,6 @@ > #define QERR_PERMISSION_DENIED \ > "Insufficient permission to perform this operation" > > -#define QERR_PROPERTY_VALUE_BAD \ > - "Property '%s.%s' doesn't take value '%s'" > - > #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ > "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" > > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c > index c34aac6ebc9..dbea4cf8e5e 100644 > --- a/hw/core/qdev-properties.c > +++ b/hw/core/qdev-properties.c > @@ -663,7 +663,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj, > break; > default: > case -EINVAL: > - error_setg(errp, QERR_PROPERTY_VALUE_BAD, > + error_setg(errp, "Property '%s.%s' doesn't take value '%s'", > object_get_typename(obj), name, value); > break; > case -ENOENT: > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index fc3ed80ef1e..bc63b80e5bd 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -4469,7 +4469,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, > int i; > > if (strlen(value) != CPUID_VENDOR_SZ) { > - error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value); > + error_setg(errp, "Property '.vendor' doesn't take value '%s'", value); > return; > } We error out unless the string has exactly CPUID_VENDOR_SZ characters. We don't tell the user, though[*]. We should! If this patch was long, I'd separate the long & mechanical from the error message improvement. Since it isn't, I suggest to make the error message improvement the patch's subject, and include the removal of QERR_PROPERTY_VALUE_BAD "while there". You choose how to structure this. [*] This is a common issue with error systems that make new error messages harder than reusing some existing message.
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h index f49ae01cdb0..a3f44fc4a1e 100644 --- a/include/qapi/qmp/qerror.h +++ b/include/qapi/qmp/qerror.h @@ -50,9 +50,6 @@ #define QERR_PERMISSION_DENIED \ "Insufficient permission to perform this operation" -#define QERR_PROPERTY_VALUE_BAD \ - "Property '%s.%s' doesn't take value '%s'" - #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \ "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")" diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index c34aac6ebc9..dbea4cf8e5e 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -663,7 +663,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj, break; default: case -EINVAL: - error_setg(errp, QERR_PROPERTY_VALUE_BAD, + error_setg(errp, "Property '%s.%s' doesn't take value '%s'", object_get_typename(obj), name, value); break; case -ENOENT: diff --git a/target/i386/cpu.c b/target/i386/cpu.c index fc3ed80ef1e..bc63b80e5bd 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -4469,7 +4469,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value, int i; if (strlen(value) != CPUID_VENDOR_SZ) { - error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value); + error_setg(errp, "Property '.vendor' doesn't take value '%s'", value); return; }
QERR_PROPERTY_VALUE_BAD definition is obsolete since 2015 (commit 4629ed1e989, "qerror: Finally unused, clean up"). Replace the two uses and drop the definition. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- include/qapi/qmp/qerror.h | 3 --- hw/core/qdev-properties.c | 2 +- target/i386/cpu.c | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-)