qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]