[pulseaudio-discuss] [PATCHv2 30/60] bluetooth: Implement org.bluez.MediaEndpoint1.SetConfiguration()

[Tanu Kaskinen]

On Tue, 2013-08-13 at 01:54 -0300, jprvita at gmail.com wrote:
> From: João Paulo Rechi Vita <jprvita at openbossa.org>
> 
> ---
>  src/modules/bluetooth/bluez5-util.c | 171 +++++++++++++++++++++++++++++++++++-
>  src/modules/bluetooth/bluez5-util.h |   5 ++
>  2 files changed, 174 insertions(+), 2 deletions(-)
> 
> diff --git a/src/modules/bluetooth/bluez5-util.c b/src/modules/bluetooth/bluez5-util.c
> index 9687997..d0428a9 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -33,6 +33,8 @@
>  #include <pulsecore/refcnt.h>
>  #include <pulsecore/shared.h>
>  
> +#include "a2dp-codecs.h"
> +
>  #include "bluez5-util.h"
>  
>  #define BLUEZ_SERVICE "org.bluez"
> @@ -359,12 +361,177 @@ fail:
>      return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
>  }
>  
> +const char *pa_bluetooth_profile_to_string(pa_bluetooth_profile_t profile) {
> +    switch(profile) {
> +        case PROFILE_A2DP_SINK:
> +            return "a2dp_sink";
> +        case PROFILE_A2DP_SOURCE:
> +            return "a2dp_source";
> +        case PROFILE_OFF:
> +            return "off";
> +    }
> +
> +    return NULL;
> +}
> +
>  static DBusMessage *endpoint_set_configuration(DBusConnection *conn, DBusMessage *m, void *userdata) {
> +    pa_bluetooth_discovery *y = userdata;
> +    pa_bluetooth_device *d;
> +    pa_bluetooth_transport *t;
> +    const char *sender, *path, *endpoint_path, *dev_path = NULL, *uuid = NULL;
> +    uint8_t *config = NULL;

config should be marked as const, because it will point to memory owned
by the DBusMessage object.

> +    int size = 0;
> +    pa_bluetooth_profile_t p = PROFILE_OFF;
> +    DBusMessageIter args, props;
>      DBusMessage *r;
>  
> -    pa_assert_se(r = dbus_message_new_error(m, BLUEZ_MEDIA_ENDPOINT_INTERFACE ".Error.NotImplemented",
> -                                            "Method not implemented"));
> +    if (!dbus_message_iter_init(m, &args) || !pa_streq(dbus_message_get_signature(m), "oa{sv}")) {
> +        pa_log_error("Invalid signature for method SetConfiguration()");
> +        goto fail2;
> +    }
> +
> +    dbus_message_iter_get_basic(&args, &path);
> +
> +    if (pa_hashmap_get(y->transports, path)) {
> +        pa_log_error("Endpoint SetConfiguration(): Transport %s is already configured.", path);
> +        goto fail2;
> +    }
> +
> +    pa_assert_se(dbus_message_iter_next(&args));
> +
> +    dbus_message_iter_recurse(&args, &props);
> +    if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
> +        goto fail;
> +
> +    /* Read transport properties */
> +    while (dbus_message_iter_get_arg_type(&props) == DBUS_TYPE_DICT_ENTRY) {
> +        const char *key;
> +        DBusMessageIter value, entry;
> +        int var;
> +
> +        dbus_message_iter_recurse(&props, &entry);
> +        dbus_message_iter_get_basic(&entry, &key);
> +
> +        dbus_message_iter_next(&entry);
> +        dbus_message_iter_recurse(&entry, &value);
> +
> +        var = dbus_message_iter_get_arg_type(&value);
> +
> +        if (pa_streq(key, "UUID")) {
> +            if (var != DBUS_TYPE_STRING) {
> +                pa_log_error("Property %s of wrong type %c", key, (char)var);
> +                goto fail;
> +            }
> +
> +            dbus_message_iter_get_basic(&value, &uuid);
> +        } else if (pa_streq(key, "Device")) {
> +            if (var != DBUS_TYPE_OBJECT_PATH) {
> +                pa_log_error("Property %s of wrong type %c", key, (char)var);
> +                goto fail;
> +            }
> +
> +            dbus_message_iter_get_basic(&value, &dev_path);
> +        } else if (pa_streq(key, "Configuration")) {
> +            DBusMessageIter array;
> +            a2dp_sbc_t *c;
> +
> +            if (var != DBUS_TYPE_ARRAY) {
> +                pa_log_error("Property %s of wrong type %c", key, (char)var);
> +                goto fail;
> +            }
> +
> +            dbus_message_iter_recurse(&value, &array);
> +            var = dbus_message_iter_get_arg_type(&array);
> +            if (var != DBUS_TYPE_BYTE) {
> +                pa_log_error("%s is an array of wrong type %c", key, (char)var);
> +                goto fail;
> +            }
> +
> +            dbus_message_iter_get_fixed_array(&array, &config, &size);
> +            c = (a2dp_sbc_t *) config;

You should check that the config size is what you expect it to be.

Also, casting the byte array to an a2dp_sbc_t pointer seems hazardous
from memory alignment point of view. I think it would be best to copy
the config to a real a2dp_sbc_t variable before accessing the config.

It would better to do the config validation later, after checking that
the UUID matches the endpoint type, because if the UUID doesn't match,
then the config is probably not what you expect it to be either.
Checking the config too early may result in less clear error messages
(it's better to complain about UUID mismatch than errors in the config).

-- 
Tanu