[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 10:30:15 -0300 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Thu, Jun 23, 2016 at 08:40:41AM -0300, Eduardo Habkost wrote:
> 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!
As I am removing the patches that remove
qdev_prop_check_globals() and the used/user_provided fields, I
will change the patch to just add the extra check in vl.c, and
not touch the test code and qdev_prop_check_globals().
I will send v3 soon.
--
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