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! -- Eduardo
