On Wed, 2013-11-20 at 10:52 -0300, jprvita at gmail.com wrote:
> From: Jo?o Paulo Rechi Vita <jprvita at gmail.com>
>
> We need to check if d->adapter is valid before dereferencing it, and
> also make sure both address strings are valid before calling pa_streq().
> ---
> src/modules/bluetooth/bluez5-util.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index c8ff219..53330ab 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -395,7 +395,9 @@ pa_bluetooth_device* pa_bluetooth_discovery_get_device_by_address(pa_bluetooth_d
> pa_assert(local);
>
> while ((d = pa_hashmap_iterate(y->devices, &state, NULL)))
> - if (pa_streq(d->address, remote) && pa_streq(d->adapter->address, local))
> + if (d->address && pa_streq(d->address, remote) &&
> + d->adapter && d->adapter->address && pa_streq(d->adapter->address, local))
> +
> return d->device_info_valid == 1 ? d : NULL;
>
> return NULL;
I don't like all this validity checking, when all that should be needed
is to check whether device_info_valid is 1.
I propose that d->device_info_valid is checked before checking anything
else. That's not sufficient, however - we also need to set
device_info_valid to -1 if the adapter is invalid (doesn't have an
address). This can be done at the end of
parse_interfaces_and_properties(), as an amendment to this code:
if (!d->adapter && d->adapter_path) {
d->adapter = pa_hashmap_get(d->discovery->adapters, d->adapter_path);
if (!d->adapter) {
pa_log_error("Device %s is child of nonexistent adapter %s", d->path, d->adapter_path);
set_device_info_valid(d, -1);
} else
set_device_info_valid(d, 1);
}
What do you think?
--
Tanu
