Hi Grzegorz,
On Thu, Aug 14, 2014, Grzegorz Kolodziejczyk wrote:
> + prop->channel = (uint16_t)channel;
This typecast seems unnecessary to me. At least I don't get any compiler
warnings/errors if I remove it.
> +static void find_remote_sdp_rec_cb(sdp_list_t *recs, int err,
> + gpointer user_data)
> +{
> + uint8_t name_len = 0;
> + uint8_t status;
> + char name_buf[256];
> + int channel = 0;
Since both name_len and channel are tied to the specific record that
you're iterating it seems wrong to me to initialize these here.
> + for ( ; recs; recs = recs->next) {
> + sdp_rec = recs->data;
> +
> + switch (sdp_rec->svclass.type) {
> + case SDP_UUID16:
> + sdp_uuid16_to_uuid128(&uuid128_sdp,
> + &sdp_rec->svclass);
> + break;
> + case SDP_UUID32:
> + sdp_uuid32_to_uuid128(&uuid128_sdp,
> + &sdp_rec->svclass);
> + break;
> + case SDP_UUID128:
> + break;
> + default:
> + error("wrong sdp uuid type");
> + goto done;
> + }
> +
> + if (!sdp_get_access_protos(sdp_rec, &protos)) {
> + channel = sdp_get_proto_port(protos, L2CAP_UUID);
> + if (channel < 0)
> + error("wrong channel");
> + }
Probably this should be here:
} else {
channel = -1;
}
> + if (!sdp_get_service_name(sdp_rec, name_buf, sizeof(name_buf)))
> + name_len = (uint8_t)strlen(name_buf);
And here:
else
name_len = 0;
Otherwise you'll end up including the name from previous records as the
name of subsequent ones which don't have a service name attribute. Also,
the (uint8_t) typecast seems unnecessary here (compiles fine for me
without it).
> +static uint8_t find_remote_sdp_rec(const bdaddr_t *addr,
> + const uint8_t *find_uuid)
> +{
> + uuid_t uuid;
> + bdaddr_t *bdaddr;
> +
> + /* from android we always get full 128bit length uuid */
> + sdp_uuid128_create(&uuid, find_uuid);
> +
> + bdaddr = malloc(sizeof(*bdaddr));
Could you tell me what's the rationale of choosing malloc vs g_malloc in
this code. In most places this c-file is using the GLib allocators and
having a mixed set is just begging for trouble (e.g. you might end up
calling g_free on something allocated with malloc, or free for something
allocated with g_malloc).
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
