Hi Arman,
> This patch changes the GATT specific linked list structure in gatt-helpers to
> bt_gatt_result and bt_gatt_iter. bt_gatt_result internally stores a linked list
> of ATT response PDUs, which get decoded on-demand by the iterator functions.
> Each iterator function operates on a specific ATT response PDU.
> ---
> src/shared/gatt-helpers.c | 577 +++++++++++++++++++++++-----------------------
> src/shared/gatt-helpers.h | 21 +-
> 2 files changed, 309 insertions(+), 289 deletions(-)
>
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 489b8c4..c237b00 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -36,69 +36,207 @@
> #define MIN(a, b) ((a) < (b) ? (a) : (b))
> #endif
>
> -struct bt_gatt_list {
> - struct bt_gatt_list *next;
> - void *data;
> -};
> +struct bt_gatt_result {
> + uint8_t opcode;
> + void *pdu;
> + uint16_t pdu_len;
> + uint16_t data_len;
> +
> + void *op; /* Discovery operation data */
>
> -struct list_ptrs {
> - struct bt_gatt_list *head;
> - struct bt_gatt_list *tail;
> + struct bt_gatt_result *next;
> };
>
> -struct bt_gatt_list *bt_gatt_list_get_next(struct bt_gatt_list *list)
> +static struct bt_gatt_result *result_create(uint8_t opcode, const void *pdu,
> + uint16_t pdu_len,
> + uint16_t data_len,
> + void *op)
> {
> - return list->next;
> + struct bt_gatt_result *result = new0(struct bt_gatt_result, 1);
> +
> + if (!result)
> + return NULL;
I now that this saves a few lines of code, but I prefer that assignment of variables is only done with "static" value.
result = new0(...
if (!result)
return NULL;
It has the visual advantage of having the NULL check right where you did the allocation.
> +
> + result->pdu = malloc(pdu_len);
> + if (!result->pdu) {
> + free(result);
> + return NULL;
> + }
> +
> + result->opcode = opcode;
> + result->pdu_len = pdu_len;
> + result->data_len = data_len;
> + result->op = op;
> +
> + memcpy(result->pdu, pdu, pdu_len);
> +
> + return result;
> }
>
> -void *bt_gatt_list_get_data(struct bt_gatt_list *list)
> +static void result_destroy(struct bt_gatt_result *result)
> {
> - return list->data;
> + struct bt_gatt_result *next;
> +
> + while (result) {
> + next = result->next;
> +
> + free(result->pdu);
> + free(result);
> +
> + result = next;
> + }
> }
>
> -static inline bool list_isempty(struct list_ptrs *list)
> +bool bt_gatt_iter_init(struct bt_gatt_iter *iter, struct bt_gatt_result *result)
> {
> - return !list->head && !list->tail;
> + if (!iter || !result)
> + return false;
> +
> + iter->result = result;
> + iter->pos = 0;
> +
> + return true;
> }
>
> -static bool list_add(struct list_ptrs *list, void *data)
> +static const uint8_t bt_base_uuid[16] = {
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
> + 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB
> +};
> +
> +static bool convert_uuid_le(const uint8_t *src, size_t len, uint8_t dst[16])
> {
> - struct bt_gatt_list *item = new0(struct bt_gatt_list, 1);
> - if (!item)
> + if (len == 16) {
> + bswap_128(src, dst);
> + return true;
> + }
> +
> + if (len != 2)
> return false;
>
> - item->data = data;
> + memcpy(dst, bt_base_uuid, sizeof(bt_base_uuid));
> + dst[2] = src[1];
> + dst[3] = src[0];
>
> - if (list_isempty(list)) {
> - list->head = list->tail = item;
> - return true;
> + return true;
> +}
> +
> +struct result_ptrs {
> + struct bt_gatt_result *head;
> + struct bt_gatt_result *tail;
> +};
> +
> +struct discovery_op {
> + struct bt_att *att;
> + uint16_t end_handle;
> + int ref_count;
> + bt_uuid_t uuid;
> + struct result_ptrs result;
Does this nested struct give you any benefit. Why not just include head and tail pointers directly.
I was actually thinking that struct bt_gatt_result will keep the tail pointer all by itself. That way it is self-contained.
Regards
Marcel
--
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
