[pulseaudio-discuss] [PATCH v4 15/41] bluetooth: Parse BlueZ 5 adapter properties

[Tanu Kaskinen]

On Thu, 2013-09-26 at 14:11 -0300, João Paulo Rechi Vita wrote:
> On Wed, Sep 25, 2013 at 7:33 AM, Tanu Kaskinen
> <tanu.kaskinen at linux.intel.com> wrote:
> > On Tue, 2013-09-24 at 19:19 -0300, João Paulo Rechi Vita wrote:
> >> On Sun, Sep 22, 2013 at 2:38 AM, Tanu Kaskinen
> >> <tanu.kaskinen at linux.intel.com> wrote:
> >> > On Sat, 2013-09-21 at 16:12 -0500, João Paulo Rechi Vita wrote:
> >> >> On Sat, Sep 21, 2013 at 6:12 AM, Tanu Kaskinen
> >> >> <tanu.kaskinen at linux.intel.com> wrote:
> >> >> > On Wed, 2013-09-18 at 16:17 -0500, jprvita at gmail.com wrote:
> >> >> >>  static void parse_interfaces_and_properties(pa_bluetooth_discovery *y, DBusMessageIter *dict_i) {
> >> >> >>      DBusMessageIter element_i;
> >> >> >>      const char *path;
> >> >> >> @@ -415,7 +474,8 @@ static void parse_interfaces_and_properties(pa_bluetooth_discovery *y, DBusMessa
> >> >> >>
> >> >> >>              pa_log_debug("Adapter %s found", path);
> >> >> >>
> >> >> >> -            /* TODO: parse adapter properties and register endpoints */
> >> >> >> +            parse_adapter_properties(a, &iface_i, false);
> >> >> >
> >> >> > If parsing fails, or if the Address property is missing, the adapter
> >> >> > should be marked as invalid.
> >> >> >
> >> >>
> >> >> We don't need to add a adapter_info_valid field, just not registering
> >> >> as an endpoint with that adapter should be enough.
> >> >
> >> > I don't think that's enough. Devices can point to adapters, so devices
> >> > that point to invalid adapters should be marked as invalid too. I think
> >> > an info_valid field also for adapters is the way to go.
> >> >
> >>
> >> There is no problem if devices point to invalid adapters, since we
> >> have not registered as endpoints we'll never receive transports for
> >> these devices, so nothing will happen.
> >
> > pa_bluetooth_discovery_get_device_by_address() will crash if a device
> > points to an adapter with NULL address.
> >
> > Actually, marking a device invalid if the adapter is invalid isn't
> > sufficient to fix pa_bluetooth_discovery_get_device_by_address(). The
> > function should check d->device_info_valid before trying to access
> > d->adapter.
> >
> 
> Ok, that's a problem in pa_bluetooth_discovery_get_device_by_address()
> then. Fixing it leaves no other problems related to the adapter being
> invalid, right?

Seems so, with the current code. However, logically it doesn't make
sense to treat devices as valid if their adapter is invalid. Future code
can easily have bugs due to this, if adapters ever become more important
(right now the adapter usage is very limited: we track the adapters only
to be able to look up devices based on the local/remote address pair).

I think it would be good to have the adapter_info_valid field, and to
treat devices with an invalid address as invalid themselves, but if you
don't want to do that, feel free to leave the code as it is.

-- 
Tanu