[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 03/10] vl: Reject invalid class names on -glo
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v2 03/10] vl: Reject invalid class names on -global |
Date: |
Thu, 23 Jun 2016 08:40:41 -0300 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Thu, Jun 23, 2016 at 09:52:12AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
>
> > Instead of just printing a warning very late, reject obviously
> > invalid -global arguments by validating the class name.
> >
> > Update test-qdev-global-props to not expect class name validation
> > to be performed in qdev_prop_check_globals().
> >
> > Reviewed-by: Markus Armbruster <address@hidden>
> > Signed-off-by: Eduardo Habkost <address@hidden>
> > ---
> > Changes v1 -> v2:
> > * Fix test-qdev-global-props unit test
> > * Simplify object_dynamic_cast() check
> > * Suggested-by: Markus Armbruster <address@hidden>
> > ---
> > hw/core/qdev-properties.c | 7 -------
> > tests/test-qdev-global-props.c | 10 ----------
> > vl.c | 20 +++++++++++++++++---
> > 3 files changed, 17 insertions(+), 20 deletions(-)
> >
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index c10edee..64e17aa 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1052,13 +1052,6 @@ int qdev_prop_check_globals(void)
> > continue;
> > }
> > oc = object_class_by_name(prop->driver);
> > - oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> > - if (!oc) {
> > - error_report("Warning: global %s.%s has invalid class name",
> > - prop->driver, prop->property);
> > - ret = 1;
> > - continue;
> > - }
> > dc = DEVICE_CLASS(oc);
> > if (!dc->hotpluggable && !prop->used) {
> > error_report("Warning: global %s.%s=%s not used",
> > diff --git a/tests/test-qdev-global-props.c b/tests/test-qdev-global-props.c
> > index 48e5b73..db77ad9 100644
> > --- a/tests/test-qdev-global-props.c
> > +++ b/tests/test-qdev-global-props.c
> > @@ -201,10 +201,8 @@ static void test_dynamic_globalprop_subprocess(void)
> > static GlobalProperty props[] = {
> > { TYPE_DYNAMIC_PROPS, "prop1", "101", true },
> > { TYPE_DYNAMIC_PROPS, "prop2", "102", true },
> > - { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", true },
> > { TYPE_UNUSED_HOTPLUG, "prop4", "104", true },
> > { TYPE_UNUSED_NOHOTPLUG, "prop5", "105", true },
> > - { TYPE_NONDEVICE, "prop6", "106", true },
> > {}
> > };
> > int all_used;
> > @@ -222,8 +220,6 @@ static void test_dynamic_globalprop_subprocess(void)
> > g_assert(props[1].used);
> > g_assert(!props[2].used);
> > g_assert(!props[3].used);
> > - g_assert(!props[4].used);
> > - g_assert(!props[5].used);
> > }
> >
> > static void test_dynamic_globalprop(void)
> > @@ -232,10 +228,8 @@ static void test_dynamic_globalprop(void)
> > g_test_trap_assert_passed();
> > g_test_trap_assert_stderr_unmatched("*prop1*");
> > g_test_trap_assert_stderr_unmatched("*prop2*");
> > - g_test_trap_assert_stderr("*Warning: global
> > dynamic-prop-type-bad.prop3 has invalid class name\n*");
> > g_test_trap_assert_stderr_unmatched("*prop4*");
> > g_test_trap_assert_stderr("*Warning: global nohotplug-type.prop5=105
> > not used\n*");
> > - g_test_trap_assert_stderr("*Warning: global nondevice-type.prop6 has
> > invalid class name\n*");
> > g_test_trap_assert_stdout("");
> > }
> >
> > @@ -246,10 +240,8 @@ static void
> > test_dynamic_globalprop_nouser_subprocess(void)
> > static GlobalProperty props[] = {
> > { TYPE_DYNAMIC_PROPS, "prop1", "101" },
> > { TYPE_DYNAMIC_PROPS, "prop2", "102" },
> > - { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103" },
> > { TYPE_UNUSED_HOTPLUG, "prop4", "104" },
> > { TYPE_UNUSED_NOHOTPLUG, "prop5", "105" },
> > - { TYPE_NONDEVICE, "prop6", "106" },
> > {}
> > };
> > int all_used;
> > @@ -267,8 +259,6 @@ static void
> > test_dynamic_globalprop_nouser_subprocess(void)
> > g_assert(props[1].used);
> > g_assert(!props[2].used);
> > g_assert(!props[3].used);
> > - g_assert(!props[4].used);
> > - g_assert(!props[5].used);
> > }
> >
>
> The rest of the patch turns a warning into an error, but the test update
> *drops* the affected test cases. Shouldn't we test the error instead?
We could test the error, but good luck linking vl.o in a unit
test.
>
> To keep my R-by, you can either do that, or mention the loss of test
> cases in the commit message.
I will mention it in the commit message. Thanks!
--
Eduardo
- [Qemu-devel] [PATCH v2 00/10] globals: Clean up validation and error checking, Eduardo Habkost, 2016/06/20
- [Qemu-devel] [PATCH v2 01/10] qdev: Don't stop applying globals on first error, Eduardo Habkost, 2016/06/20
- [Qemu-devel] [PATCH v2 02/10] qdev: Eliminate qemu_add_globals() function, Eduardo Habkost, 2016/06/20
- [Qemu-devel] [PATCH v2 05/10] machine: Add machine_register_compat_props() function, Eduardo Habkost, 2016/06/20
- [Qemu-devel] [PATCH v2 08/10] qdev: Eliminate GlobalProperty 'used' and 'user_provided' fields, Eduardo Habkost, 2016/06/20