Re: [PATCH v1 2/2] service-find command for new kernel method usage

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

 



Hi Jakub,

On Wed, Oct 29, 2014, Jakub Pawlowski wrote:
> +static void uuid_to_uuid128(uuid_t *uuid128, const uuid_t *uuid);

Please just move up this function instead of creating a
forward-declaration for it.

> +static void cmd_find_service(struct mgmt *mgmt, uint16_t index, int argc, char **argv)
> +{
> +	struct mgmt_cp_start_service_discovery *cp;
> +	struct mgmt_uuid_filter *filter;
> +	uint8_t type;
> +	int opt;
> +	int total_size = sizeof(struct mgmt_cp_start_service_discovery) + sizeof(struct mgmt_uuid_filter);
> +	uuid_t uuid;
> +	uint128_t uint128;
> +	uuid_t uuid128;
> +
> +	if (index == MGMT_INDEX_NONE)
> +		index = 0;
> +
> +	type = 0;
> +	hci_set_bit(BDADDR_BREDR, &type);
> +	hci_set_bit(BDADDR_LE_PUBLIC, &type);
> +	hci_set_bit(BDADDR_LE_RANDOM, &type);
> +
> +	cp = malloc(total_size);
> +	if(cp == NULL) {

Missing space after 'if', however I wonder is there any point to do
dynamic allocation here since you always just have just one filter
entry. Having the send buffer on the stack might create simpler code
here.

> +		fprintf(stderr, "Unable to allocate memory for mgmt_cp_start_service_discovery structure.\n");
> +	}

Wouldn't you need to return from the function in this case?

> +	memset(cp, 0, total_size);
> +
> +	filter = cp->filter;
> +
> +	if(argc == 1) {

Missing space after 'if'.

> +	if (mgmt_send(mgmt, MGMT_OP_START_SERVICE_DISCOVERY, index, total_size, cp,
> +						find_service_rsp, cp, NULL) == 0) {
> +		free(cp);
> +		fprintf(stderr, "Unable to send start_service_discovery cmd\n");
> +		exit(EXIT_FAILURE);
> +	}
> +}

mgmt_send allocates its own buffer for the send parameters so there's no
need for you to keep your own buffer around until the command completion
callback (where all you do is free it). However, this whole issue would
go away if you'd just use a stack variable.

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