Hi Michael,
On Tue, Mar 31, 2015 at 8:23 PM, Michael Janssen <jamuraa@xxxxxxxxxxxx> wrote:
> The bt_ad structure provides an abstraction for easy translation of defined
> Advertisement Data fields into the resulting raw bytes needed by the Bluetooth
> HCI LE Set Advertising Data command.
> ---
> Makefile.am | 1 +
> src/shared/ad.c | 346 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/ad.h | 60 ++++++++++
> 3 files changed, 407 insertions(+)
> create mode 100644 src/shared/ad.c
> create mode 100644 src/shared/ad.h
>
> diff --git a/Makefile.am b/Makefile.am
> index 676d929..e144428 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -111,6 +111,7 @@ shared_sources = src/shared/io.h src/shared/timeout.h \
> src/shared/uhid.h src/shared/uhid.c \
> src/shared/pcap.h src/shared/pcap.c \
> src/shared/btsnoop.h src/shared/btsnoop.c \
> + src/shared/ad.h src/shared/ad.c \
> src/shared/att-types.h \
> src/shared/att.h src/shared/att.c \
> src/shared/gatt-helpers.h src/shared/gatt-helpers.c \
> diff --git a/src/shared/ad.c b/src/shared/ad.c
> new file mode 100644
> index 0000000..8e38d26
> --- /dev/null
> +++ b/src/shared/ad.c
> @@ -0,0 +1,346 @@
> +/*
> + *
> + * BlueZ - Bluetooth protocol stack for Linux
> + *
> + * Copyright (C) 2015 Google Inc.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include "src/shared/ad.h"
> +
> +#include "src/shared/queue.h"
> +#include "src/shared/util.h"
> +
> +struct bt_ad {
> + int ref_count;
> + struct queue *service_uuids;
> + struct queue *manufacturer_data;
> + struct queue *solicit_uuids;
> + struct queue *service_data;
Did you thought about using a single queue and carrying a type in the
struct data? Perhaps it would be simpler, but in the other hand it may
consume more memory per entry but usually we don't have many entries.
> +};
> +
> +struct uuid_tagged_data {
> + bt_uuid_t uuid;
> + uint8_t *data;
> + size_t len;
> +};
> +
> +struct manufacturer_tagged_data {
> + uint16_t manufacturer_id;
> + uint8_t *data;
> + size_t len;
> +};
I would remove tagged term from these structs.
> +void bt_ad_clear_service_uuid(struct bt_ad *ad)
> +{
For public functions please add a NULL check first.
> + queue_destroy(ad->service_uuids, free);
> +
> + ad->service_uuids = queue_new();
I would prefer queue_remove_all instead of queue_destroy+queue_new to cleanup.
> +bool bt_ad_add_manufacturer_data(struct bt_ad *ad, uint16_t manufacturer_id,
> + void *data, size_t len)
> +{
> + struct manufacturer_tagged_data *new_data;
> +
> + if (!ad)
> + return false;
> +
> + new_data = new0(struct manufacturer_tagged_data, 1);
> + if (!new_data)
> + return false;
> +
> + new_data->manufacturer_id = manufacturer_id;
> +
> + new_data->data = malloc(len);
> + if (!new_data->data) {
> + free(new_data);
> + return false;
> + }
> +
> + memcpy(new_data->data, data, len);
How about having a memdup?
--
Luiz Augusto von Dentz
--
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
