GNOME Bugzilla – Bug 787551
Factor out some duplicated code in GParamSpec validation
Last modified: 2017-09-11 20:07:41 UTC
There is some duplicated code among g_object_class_install_property() g_object_class_install_properties() g_object_interface_install_property() where they validate the GParamSpec and whether the class has all the required methods. I'll attach a patch set that takes care of this.
Created attachment 359540 [details] [review] GObjectClass: Validate installing property/properties in the same way Then we'll be able to factor out duplicated code.
Created attachment 359541 [details] [review] GObjectClass: extract class type and parent type at the beginning
Created attachment 359542 [details] [review] validate_and_install_class_property(): Factor out function to add a new property to a class This was duplicated between g_object_class_install_property() and g_object_class_install_properties().
Created attachment 359543 [details] [review] install_property_internal(): Propagate failure when installing duplicated properties
Created attachment 359544 [details] [review] g_object_interface_install_property(): Do interface-specific validations first
Created attachment 359545 [details] [review] validate_and_install_property(): Validate presence of get/set_property methods last
Created attachment 359546 [details] [review] validate_and_install_property(): Check pspec-specific fields in the same order as g_object_interface_install_property()
Created attachment 359547 [details] [review] validate_pspec_to_install(): Factor out function to validate a GParamSpec This was duplicated also in g_object_interface_install_property(). Now, validations specific to classes happen in validate_and_install_class_property() - specifically, the checks for the presence of the get_property() and set_property() methods.
Review of attachment 359540 [details] [review]: looks good to me
Review of attachment 359541 [details] [review]: sure
Review of attachment 359542 [details] [review]: ok
Review of attachment 359543 [details] [review]: not sure there's much to be won here - pretty much all of these errors just mean 'this code is busted'. But anyway...
Review of attachment 359544 [details] [review]: right
Review of attachment 359545 [details] [review]: ok
Review of attachment 359546 [details] [review]: ok
Review of attachment 359547 [details] [review]: ::: gobject/gobject.c @@ +559,3 @@ + return FALSE; + } + Loose the {} here, to match glib/gtk+ style @@ +768,3 @@ + { + return; + } same here
These all look good to me too. Thanks for the patches!
Pushed everything after removing the braces around single-line returns. Thanks! About the validation - I wanted to prevent the class->* from being modified if the property didn't actually get inserted because it was in error.