Re: [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Vinicius,

On 01/22/2013 03:43 PM, Vinicius Costa Gomes wrote:
> Parse the essential arguments in the message, in this case only the
> file descriptor, and register the modem if it is not already
> registered. This is necessary because in some cases, we may receive a
> NewConnection call, and the SDP process is still taking place.
> ---
>   plugins/hfp_hf_bluez5.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index 74ca570..39853e3 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -24,6 +24,8 @@
>   #endif
>
>   #include<errno.h>
> +#include<unistd.h>
> +
>   #include<glib.h>
>
>   #include<gdbus.h>
> @@ -124,11 +126,50 @@ static struct ofono_modem_driver hfp_driver = {
>   static DBusMessage *profile_new_connection(DBusConnection *conn,
>   					DBusMessage *msg, void *user_data)
>   {
> +	struct ofono_modem *modem;
> +	DBusMessageIter iter;
> +	GDBusProxy *proxy;
> +	DBusMessageIter entry;
> +	const char *device, *alias;
> +	int fd;
> +
>   	DBG("Profile handler NewConnection");
>
> -	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
> -					".NotImplemented",
> -					"Implementation not provided");
> +	if (dbus_message_iter_init(msg,&entry) == FALSE)
> +		goto error;
> +
> +	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
> +		goto error;
> +
> +	dbus_message_iter_get_basic(&entry,&device);
> +
> +	proxy = g_hash_table_lookup(devices_proxies, device);
> +	if (proxy == NULL)
> +		goto error;

So we return 'Invalid Arguments' even if we failed to lookup the proxy 
in the hash?  Lets be a bit nicer here

> +
> +	if (g_dbus_proxy_get_property(proxy, "Alias",&iter) == FALSE)
> +		alias = "unknown";
> +	else
> +		dbus_message_iter_get_basic(&iter,&alias);

This looks suspicious, under what circumstances are we missing the alias?

> +
> +	dbus_message_iter_next(&entry);
> +	if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
> +		goto error;
> +
> +	dbus_message_iter_get_basic(&entry,&fd);
> +	if (fd<  0)
> +		goto error;
> +
> +	modem = modem_register(device, alias);
> +	if (modem == NULL)
> +		goto error;
> +
> +	return NULL;
> +
> +error:
> +	close(fd);
> +	return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
> +					"Invalid arguments in method call");
>   }
>
>   static DBusMessage *profile_release(DBusConnection *conn,

Regards,
-Denis