Hi Szymon,
On Tue, Nov 16, 2010, Szymon Janc wrote:
> +gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
> + uint8_t *randomizer)
Looks like the continuation line should be indented a bit more. Basicly
indent at least past the ( on the above line and as much as possible as
long as the entire line stays under 79 columns. I think I saw other
places with the same issue so please fix those too.
> +void device_set_oob_data(struct btd_device *device, uint8_t *hash,
> + uint8_t *randomizer);
> +gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
> + uint8_t *randomizer);
> +gboolean device_has_oob_data(struct btd_device *device);
I suppose you could just use get_oob_data(dev, NULL, NULL) instead of
having a separate has_oob_data function.
> +gboolean device_request_oob_data(struct btd_device *device, void *cb);
Why is the callback type void* instead of having a well defined
signature?
> +void device_set_local_auth_cap(struct btd_device *device, uint8_t auth,
> + uint8_t cap);
> +void device_get_local_auth_cap(struct btd_device *device, uint8_t *auth,
> + uint8_t *cap);
This local_cap/auth in struct device had me wondering a little bit since
it felt like that should actually be in struct adapter, however this is
really per adapter-device pair data in which case it should be fine,
right?
> +static void btd_event_io_cap_reply(struct btd_device *device)
> +{
> + io_capability_reply_cp cp;
> + int dev;
> + struct btd_adapter *adapter = device_get_adapter(device);
> + uint16_t dev_id = adapter_get_dev_id(adapter);
> +
> + dev = hci_open_dev(dev_id);
That's a no no. Only hciops should use raw HCI sockets anymore. If you
need to do something like this you'll need to add a new adapter_ops
callback and send your HCI command through that.
> {
> struct btd_adapter *adapter;
> struct btd_device *device;
> struct agent *agent = NULL;
> uint8_t agent_cap;
> int err;
> + uint8_t cap;
> + uint8_t auth;
These can be on the same line.
> + if (!plugin || !plugin->local_data_read|| !plugin->plugin_deactivated
> + || !plugin->request_remote_data
> + || active_plugin == plugin)
> + return;
When you split lines the break should be after || && etc. In this case
you could also consider splitting this up into multiple if-statements
for better readability.
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
