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 <[email protected]> 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 <[email protected]> > > > Signed-off-by: Eduardo Habkost <[email protected]> > > > --- > > > Changes v1 -> v2: > > > * Fix test-qdev-global-props unit test > > > * Simplify object_dynamic_cast() check > > > * Suggested-by: Markus Armbruster <[email protected]> > > > --- > > > 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
