Return-Path: From: Szymon Janc To: Grzegorz Kolodziejczyk Cc: linux-bluetooth@vger.kernel.org Subject: Re: [PATCH 03/10] android/map-client: Add support for get remote MAS instances Date: Mon, 13 Oct 2014 22:19:09 +0200 Message-ID: <1609087.iuvzY7v4pk@athlon> In-Reply-To: <1412858714-2845-4-git-send-email-grzegorz.kolodziejczyk@tieto.com> References: <1412858714-2845-1-git-send-email-grzegorz.kolodziejczyk@tieto.com> <1412858714-2845-4-git-send-email-grzegorz.kolodziejczyk@tieto.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Sender: linux-bluetooth-owner@vger.kernel.org List-ID: On Thursday 09 October 2014 14:45:07 Grzegorz Kolodziejczyk wrote: > This allows to get remote mas instances. In case of wrong sdp record > fail status will be returned in notification. > --- > android/map-client.c | 124 > ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 123 > insertions(+), 1 deletion(-) > > diff --git a/android/map-client.c b/android/map-client.c > index 1001b36..adeae4b 100644 > --- a/android/map-client.c > +++ b/android/map-client.c > @@ -30,21 +30,143 @@ > #include > #include > > +#include "lib/sdp.h" > +#include "lib/sdp_lib.h" > +#include "src/sdp-client.h" > + > #include "ipc.h" > #include "lib/bluetooth.h" > #include "map-client.h" > #include "src/log.h" > #include "hal-msg.h" > +#include "ipc-common.h" > +#include "utils.h" > +#include "src/shared/util.h" > > static struct ipc *hal_ipc = NULL; > static bdaddr_t adapter_addr; > > +static int fill_mce_inst(void *buf, int32_t id, int32_t scn, int32_t > msg_type, + const void *name, uint8_t name_len) > +{ > + struct hal_map_client_mas_instance *inst = buf; > + > + inst->id = id; > + inst->scn = scn; > + inst->msg_types = msg_type; > + inst->name_len = name_len; > + > + if (name_len) > + memcpy(inst->name, name, name_len); > + > + return sizeof(*inst) + name_len; > +} > + > +static void map_client_sdp_search_cb(sdp_list_t *recs, int err, gpointer > data) +{ > + uint8_t buf[IPC_MTU]; > + struct hal_ev_map_client_remote_mas_instances *ev = (void *) buf; > + bdaddr_t *dst = data; > + sdp_list_t *list, *protos; > + uint8_t status; > + int32_t id, scn, msg_type, name_len, num_instances = 0; > + char *name; > + size_t size; > + > + size = sizeof(*ev); > + bdaddr2android(dst, &ev->bdaddr); > + > + if (err < 0) { > + error("mce: Unable to get SDP record: %s", strerror(-err)); > + status = HAL_STATUS_FAILED; > + goto fail; > + } > + > + for (list = recs; list != NULL; list = list->next) { > + sdp_record_t *rec = list->data; > + sdp_data_t *data; > + > + data = sdp_data_get(rec, SDP_ATTR_MAS_INSTANCE_ID); > + if (data) { > + id = data->val.uint8; > + } else { > + error("mce: cannot get mas instance id"); > + status = HAL_STATUS_FAILED; > + goto fail; I'm not sure if we should fail here. Lets just skip record (we can leave error message) and continue with search. Also make it like: if (!data) {error(); continue;}; Not need for if-else > + } > + > + data = sdp_data_get(rec, SDP_ATTR_SVCNAME_PRIMARY); > + if (data) { > + name = data->val.str; > + name_len = data->unitSize; > + } else { > + error("mce: cannot get mas instance name"); > + status = HAL_STATUS_FAILED; > + goto fail; > + } > + > + data = sdp_data_get(rec, SDP_ATTR_SUPPORTED_MESSAGE_TYPES); > + if (data) { > + msg_type = data->val.uint8; > + } else { > + error("mce: cannot get mas instance msg type"); > + status = HAL_STATUS_FAILED; > + goto fail; > + } > + > + if (!sdp_get_access_protos(rec, &protos)) { > + scn = sdp_get_proto_port(protos, RFCOMM_UUID); > + > + sdp_list_foreach(protos, > + (sdp_list_func_t) sdp_list_free, NULL); > + sdp_list_free(protos, NULL); > + } else { > + error("mce: cannot get mas instance rfcomm channel"); > + status = HAL_STATUS_FAILED; > + goto fail; > + } > + > + size += fill_mce_inst(buf + size, id, scn, msg_type, name, > + name_len); > + num_instances++; > + } > + > + status = HAL_STATUS_SUCCESS; Please check if we are expected to return error if no instances were found. > + > +fail: > + ev->num_instances = num_instances; > + ev->status = status; > + > + ipc_send_notif(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT, > + HAL_EV_MAP_CLIENT_REMOTE_MAS_INSTANCES, size, buf); > + > + free(dst); > +} > + > static void handle_get_instances(const void *buf, uint16_t len) > { > + const struct hal_cmd_map_client_get_instances *cmd = buf; > + uint8_t status; > + bdaddr_t *dst; > + uuid_t uuid; > + > DBG(""); > > + dst = new0(bdaddr_t, 1); Please check if allocation failed. > + android2bdaddr(&cmd->bdaddr, dst); > + > + sdp_uuid16_create(&uuid, MAP_MSE_SVCLASS_ID); > + > + if (bt_search_service(&adapter_addr, dst, &uuid, > + map_client_sdp_search_cb, dst, NULL, 0)) { Just a hint: you can pass free as destroy function here instead of taking care of that in callback. > + error("mce: Failed to search SDP details"); > + status = HAL_STATUS_FAILED; > + free(dst); > + } > + > + status = HAL_STATUS_SUCCESS; In case of error status is overwritten. > ipc_send_rsp(hal_ipc, HAL_SERVICE_ID_MAP_CLIENT, > - HAL_OP_MAP_CLIENT_GET_INSTANCES, HAL_STATUS_FAILED); > + HAL_OP_MAP_CLIENT_GET_INSTANCES, status); > } > > static const struct ipc_handler cmd_handlers[] = { -- Szymon K. Janc szymon.janc@gmail.com