Re: [PATCH 1/3] bluez5: Add bt_register_profile_full()

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4053 bytes --]

Hi Vinicius,

On 04/22/2013 12:54 PM, Vinicius Costa Gomes wrote:
> This function makes it easy to set up optional information in the profile
> service record.
> ---
>   plugins/bluez5.c | 40 +++++++++++++++++++++++++++++++++-------
>   plugins/bluez5.h | 10 ++++++++++
>   2 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/plugins/bluez5.c b/plugins/bluez5.c
> index 0f997da..48d8bf2 100644
> --- a/plugins/bluez5.c
> +++ b/plugins/bluez5.c
> @@ -90,9 +90,10 @@ done:
>   	dbus_message_unref(reply);
>   }
>
> -int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
> -					uint16_t version, const char *name,
> -					const char *object, const char *role)
> +int bt_register_profile_full(DBusConnection *conn,
> +					const char *uuid, uint16_t version,
> +					const char *name, const char *object,
> +					struct bt_profile_info *optional)

We are now getting too many variations.  We currently have 4 callers of 
bt_register_profile (3 the simple function, 1 of _role).  Now we have 4 
callers and 3 variations.  This is too much.  Lets just make 
bt_register_profile accept the full set of arguments and change everyone 
over.

Also, I don't like introducing a struct bt_profile_info for this.  Just 
add the extra arguments you need and accept NULL where needed.

>   {
>   	DBusMessageIter iter, dict;
>   	DBusPendingCall *c;
> @@ -111,9 +112,19 @@ int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
>   	ofono_dbus_dict_append(&dict, "Name", DBUS_TYPE_STRING,&name);
>   	ofono_dbus_dict_append(&dict, "Version", DBUS_TYPE_UINT16,&version);
>
> -	if (role)
> -		ofono_dbus_dict_append(&dict, "Role", DBUS_TYPE_STRING,&role);
>
> +	if (optional == NULL)
> +		goto done;
> +
> +	if (optional->role)
> +		ofono_dbus_dict_append(&dict, "Role", DBUS_TYPE_STRING,
> +							&optional->role);
> +
> +	if (optional->features)
> +		ofono_dbus_dict_append(&dict, "Features", DBUS_TYPE_UINT16,
> +							&optional->features);
> +
> +done:
>   	dbus_message_iter_close_container(&iter,&dict);
>
>   	if (!dbus_connection_send_with_reply(conn, msg,&c, -1)) {
> @@ -130,12 +141,27 @@ int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
>   	return 0;
>   }
>
> +int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
> +					uint16_t version, const char *name,
> +					const char *object, const char *role)
> +{
> +	struct bt_profile_info optional;
> +
> +	memset(&optional, 0, sizeof(optional));
> +
> +	optional.role = role;
> +
> +	return bt_register_profile_full(conn, uuid, version, name,
> +							object,&optional);
> +}
> +
> +
>   int bt_register_profile(DBusConnection *conn, const char *uuid,
>   					uint16_t version, const char *name,
>   							const char *object)
>   {
> -	return bt_register_profile_with_role(conn, uuid, version, name, object,
> -									NULL);
> +	return bt_register_profile_full(conn, uuid, version, name,
> +							object, NULL);
>   }
>
>   void bt_unregister_profile(DBusConnection *conn, const char *object)
> diff --git a/plugins/bluez5.h b/plugins/bluez5.h
> index 5b8c1f5..8118f1f 100644
> --- a/plugins/bluez5.h
> +++ b/plugins/bluez5.h
> @@ -29,10 +29,20 @@
>   #define HFP_HS_UUID	"0000111e-0000-1000-8000-00805f9b34fb"
>   #define HFP_AG_UUID	"0000111f-0000-1000-8000-00805f9b34fb"
>
> +struct bt_profile_info {
> +	const char *role;
> +	uint16_t features;
> +};
> +
>   int bt_register_profile_with_role(DBusConnection *conn, const char *uuid,
>   					uint16_t version, const char *name,
>   					const char *object, const char *role);
>
> +int bt_register_profile_full(DBusConnection *conn,
> +					const char *uuid, uint16_t version,
> +					const char *name, const char *object,
> +					struct bt_profile_info *optional);
> +
>   int bt_register_profile(DBusConnection *conn, const char *uuid,
>   					uint16_t version, const char *name,
>   					const char *object);

Regards,
-Denis