Hi Claudio,
On Fri, Mar 28, 2014, Claudio Takahasi wrote:
> This patch adds a testing purpose only characteristic descriptor to
> allow reading and writing descriptors "Value" property.
> ---
> tools/gatt-service.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 43 insertions(+), 7 deletions(-)
I've applied the first two patches but noticed something with this one:
> +static gboolean register_characteristic(const char *chr_uuid,
> const uint8_t *value, int vlen,
> const char **props,
> + const char *desc_uuid,
> const char *service_path)
> {
> struct characteristic *chr;
> static int id = 1;
> - char *path;
> + char *chr_path, *desc_path;
> gboolean ret = TRUE;
>
> - path = g_strdup_printf("%s/characteristic%d", service_path, id++);
> + chr_path = g_strdup_printf("%s/characteristic%d", service_path, id++);
>
> chr = g_new0(struct characteristic, 1);
>
> - chr->uuid = g_strdup(uuid);
> + chr->uuid = g_strdup(chr_uuid);
> chr->value = g_memdup(value, vlen);
> chr->vlen = vlen;
> - chr->path = path;
> + chr->path = chr_path;
> chr->props = props;
>
> - if (!g_dbus_register_interface(conn, path, GATT_CHR_IFACE,
> + if (!g_dbus_register_interface(connection, chr_path, GATT_CHR_IFACE,
> NULL, NULL, chr_properties,
> chr, chr_iface_destroy)) {
> printf("Couldn't register characteristic interface\n");
> + return FALSE;
> + }
> +
> + if (!desc_uuid)
> + return ret;
> +
> + desc_path = g_strdup_printf("%s/descriptor%d", chr_path, id++);
> + if (!g_dbus_register_interface(connection, desc_path,
> + GATT_DESCRIPTOR_IFACE,
> + NULL, NULL, desc_properties,
> + g_strdup(desc_uuid), g_free)) {
> + printf("Couldn't register descriptor interface\n");
> + g_dbus_unregister_interface(connection, chr_path,
> + GATT_CHR_IFACE);
> + g_free(desc_path);
> ret = FALSE;
> }
>
Looks like you're leaking desc_path here if register_interface succeeds.
In general it seems a bit unnecessary to go for a dynamically allocated
approach since you only need the variable in the same function. Probably
a stack variable and snprintf would do an equally good or better job.
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
