Re: [PATCH v6 4/6] Add D-Bus OOB plugin

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Szymon,

On Thu, Mar 24, 2011, Szymon Janc wrote:
> +static GDBusMethodTable oob_methods[] = {
> +	{"AddRemoteOobData",	"sayay", "",	 add_remote_data},
> +	{"RemoveRemoteOobData",	"s",	 "",	 remove_remote_data},
> +	{"ReadLocalOobData",	"",	 "ayay", read_local_data,
> +						 G_DBUS_METHOD_FLAG_ASYNC},
> +	{ }
> +};

You're using mixing spaces and tabs for indentation within this table.
Just use table if you want to align the columns.

> +static void oob_remove(struct btd_adapter *adapter)
> +{
> +	local_data_read(adapter, NULL, NULL);
> +
> +	g_dbus_unregister_interface(connection, adapter_get_path(adapter),
> +							OOB_INTERFACE);
> +}
> +
> +
> +static struct btd_adapter_driver oob_driver = {

There should never be the need to have two consecutive empty lines.
Please remove one.

> +	while ((match = g_slist_next(oob_requests))) {
> +		oob_request = match->data;
> +		oob_remove(oob_request->adapter);
> +	}

This loop looks flawed to me. Why are you skipping the first element in
the list? It also took some time to see that you don't have an infinite
loop here as it's a quite indirect way that the elements get removed.
Furthermore, you've got functions called "local_data_read" and
"read_local_data". Do you think someone would immediately understand the
difference from those names? Maybe local_data_read could be named
read_local_data_complete or read_local_data_cb or something similar?

Johan