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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux