Re: [Qemu-devel] [PATCH for-2.9 1/2] qom: Remove release function from class properties

[Eduardo Habkost]

On Thu, Nov 17, 2016 at 01:33:34PM +0100, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
> 
> > The release functions are never called for class properties, and
> > their semantics aren't even defined clearly (should the release
> > function be called when an instance is destroyed, or when a class
> > is destroyed?). Remove the unused functionality.
> >
> > Signed-off-by: Eduardo Habkost <address@hidden>
[...]
> >      g_hash_table_insert(klass->properties, g_strdup(name), prop);
> > @@ -1808,7 +1806,6 @@ void object_class_property_add_str(ObjectClass 
> > *klass, const char *name,
> >      object_class_property_add(klass, name, "string",
> >                                get ? property_get_str : NULL,
> >                                set ? property_set_str : NULL,
> > -                              property_release_str,
> >                                prop, &local_err);
> >      if (local_err) {
> >          error_propagate(errp, local_err);
> 
> Here, you drop property_release_str(), which calls g_free().  Assuming
> you claim that it's never called is correct, this is again no functional
> change.  But that begs the question why not freeing the stuff
> property_release_str() frees is correct.  Can you explain?

property_release_str() frees prop. The only right moment to free
prop is on class destruction or class property removal, but we
have no mechanisms for class destruction or class property
removal today.

> 
> > @@ -1897,7 +1894,6 @@ void object_class_property_add_bool(ObjectClass 
> > *klass, const char *name,
> >      object_class_property_add(klass, name, "bool",
> >                                get ? property_get_bool : NULL,
> >                                set ? property_set_bool : NULL,
> > -                              property_release_bool,
> >                                prop, &local_err);
> >      if (local_err) {
> >          error_propagate(errp, local_err);
> > @@ -1985,7 +1981,6 @@ void object_class_property_add_enum(ObjectClass 
> > *klass, const char *name,
> >      object_class_property_add(klass, name, typename,
> >                                get ? property_get_enum : NULL,
> >                                set ? property_set_enum : NULL,
> > -                              property_release_enum,
> >                                prop, &local_err);
> >      if (local_err) {
> >          error_propagate(errp, local_err);
> > @@ -2082,7 +2077,6 @@ void object_class_property_add_tm(ObjectClass *klass, 
> > const char *name,
> >  
> >      object_class_property_add(klass, name, "struct tm",
> >                                get ? property_get_tm : NULL, NULL,
> > -                              property_release_tm,
> >                                prop, &local_err);
> >      if (local_err) {
> >          error_propagate(errp, local_err);
> 
> Likewise.

Same as above.

> [...]

-- 
Eduardo