[pulseaudio-discuss] [PATCH next v0 7/8] bluetooth: Use transport array instead of hashmap for devices

Tanu Kaskinen tanuk at iki.fi
Wed Dec 5 20:31:21 PST 2012


On Wed, 2012-12-05 at 17:23 +0100, Mikel Astiz wrote:
> @@ -169,13 +168,15 @@ static void device_free(pa_bluetooth_device *d) {
>  
>      pa_assert(d);
>  
> -    while ((t = pa_hashmap_steal_first(d->transports))) {
> +    for (i = 0; i < PA_BLUETOOTH_PROFILE_COUNT; i ++) {

Extra space in "i ++".

> +        if ((t = d->transports[i]) == NULL)

!pointer instead of pointer == NULL.

>  pa_bluetooth_transport* pa_bluetooth_device_get_transport(pa_bluetooth_device *d, enum profile profile) {
> -    pa_bluetooth_transport *t;
> -    void *state = NULL;
> -
>      pa_assert(d);
>  
> -    while ((t = pa_hashmap_iterate(d->transports, &state, NULL)))
> -        if (t->profile == profile)
> -            return t;
> +    if (profile == PROFILE_OFF)
> +        return NULL;
>  
> -    return NULL;
> +    return d->transports[profile];
>  }

This whole function is now redundant, because module-blueooth-device can
just directly look up the transport from d->transports. The only extra
thing that this function does is that it returns NULL for PROFILE_OFF,
but that feature is not needed by module-bluetooth-device.

>  
>  bool pa_bluetooth_device_any_audio_connected(const pa_bluetooth_device *d) {
> @@ -1186,10 +1183,16 @@ static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage
>  
>      sender = dbus_message_get_sender(m);
>  
> +    if (d->transports[p] != NULL) {
> +        pa_log_warn("Cannot configure transport %s because profile %d is already used", path, p);
> +        goto fail;
> +    }

Cosmetic: I think this check would be better to be before the sender
assignment.

Also, if this happens, isn't it a bug in BlueZ? Upgrading the log
message from warning to error would be warranted in that case.

-- 
Tanu



More information about the pulseaudio-discuss mailing list