Bug 30834

I did a patch too which was more ambitious and also has some testcases along with it.  Attaching for posterity.

Created attachment 42473 [details] [review]
fix handling of properties from multiple interfaces

Created attachment 42478 [details] [review]
Another take at the problem (Updated with clearer meaning for the restrictions in lookup_property_name)

Comment on attachment 42473 [details] [review]
fix handling of properties from multiple interfaces


>lookup_object_info_by_iface_cb() incorrectly checked only the first
>property when looking up the object info for a given interface, causing
>secondary interfaces to fail lookup, and thus Get/Set/GetAll requests
>for the secondary interfaces to fail.

Ouch.

>If legacy property access was disabled, lookup_property_name() should
>not have returned properties that did not match the requested interface.
>
>Third, in _dbus_gutils_wincaps_to_uscore(), a '-' should be replaced
>with a '_' so that requests for "foobar-baz" get converted into
>"foobar_baz" to match GObject property names used internally.

This is a big change actually...I seem to recall making a similar change before and backing it out.

>+  else
>     {
>-      lookup_data->info = info;
>-      lookup_data->iface_type = gtype;
>+      const char *propsig;
>+
>+      /* Iterate over properties until we find one with the requested interface */

It's a layering violation to do this inside lookup_object_info_by_iface_cb; it's currently not property-specific.  We may need to add an 

enum {
  LOOKUP_MODE_NONE,
  LOOKUP_MODE_PROPERTY
} LookupMode; ?

>+          propsig = property_iterate (propsig, object_info->format_version, &piface, &pname, &puscore, &access_type);
>+          if (g_strcmp0 (piface, wincaps_propiface) == 0 &&
>+              (g_strcmp0 (pname, requested_propname) == 0 ||
>+               g_strcmp0 (puscore, requested_propname) == 0 ||
>+               g_strcmp0 (pname, uscore_name) == 0 ||
>+               g_strcmp0 (puscore, uscore_name) == 0))
>+            {
>+              found = TRUE;
>+              break;
>+            }
>+        }

This one might be a candidate for a helper function?

So for NetworkManager we fixed our code to not have multiple interfaces in the same XML; so feel free to close this bug if you like.  If we don't plan to fix this (since it's pretty clearly broken and has been forever) then perhaps the binding tool should error out if there are multiple interfaces in the same XML.

Just going to close this since we don't care much about it anymore, and I'm sure there are higher priority things to fix.