confusion about libs/sdp.c - sdp_service_search_req()

the documentation for sdp_service_search_req() states that it outputs an sdp_list_t * of sdp_record_t * (list of sdp service records)

reading through the code, it appears that it actually returns a list of uint32_t * data structures.

line 2721:

        extract_record_handle_seq(pdata, rsp, rec_count, &scanned);

line 2523:
static void extract_record_handle_seq(char *pdu, sdp_list_t **seq, int count, int *scanned)

{
    sdp_list_t *pSeq = *seq;
    char *pdata = pdu;
    int n;
 
    for (n = 0; n < count; n++) {
        uint32_t *pSvcRec = (uint32_t *) malloc(sizeof(uint32_t));
        *pSvcRec = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);
        pdata += sizeof(uint32_t);
        *scanned += sizeof(uint32_t);
    }
    *seq = pSeq;
}

I think this will cause segfaults when you try to sdp_record_free the records in the list (it does for me).

perhaps it shold read something more like:

        sdp_record_t *pSvcRec = (sdp_record_t*) malloc(sizeof(sdp_record_t));
        pSvcRec->attrlist = pSvcRec->pattern = 0;
        pSvcRec->handle = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);

Hi Albert,

Quoted 28 lines

the documentation for sdp_service_search_req() states that it outputs an sdp_list_t * of sdp_record_t * (list of sdp service records)

reading through the code, it appears that it actually returns a list of uint32_t * data structures.

line 2721:

        extract_record_handle_seq(pdata, rsp, rec_count, &scanned);

line 2523:
static void extract_record_handle_seq(char *pdu, sdp_list_t **seq, int count, int *scanned)

{
    sdp_list_t *pSeq = *seq;
    char *pdata = pdu;
    int n;
 
    for (n = 0; n < count; n++) {
        uint32_t *pSvcRec = (uint32_t *) malloc(sizeof(uint32_t));
        *pSvcRec = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);
        pdata += sizeof(uint32_t);
        *scanned += sizeof(uint32_t);
    }
    *seq = pSeq;
}

Quoted 8 lines

I think this will cause segfaults when you try to sdp_record_free the records in the list (it does for me).

perhaps it shold read something more like:

        sdp_record_t *pSvcRec = (sdp_record_t*) malloc(sizeof(sdp_record_t));
        pSvcRec->attrlist = pSvcRec->pattern = 0;
        pSvcRec->handle = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);

Steve, any thoughts on this one?

Regards

Marcel

Hi Albert, Marcel,

The documentation is incorrect: the correct behaviour of sdp_service_search_req() is that it returns a list of service record handles --- you then use sdp_search_attr_req() to fetch attributes of interest. Alternatively, you can use sdp_search_attr_req() to perform a combined search.

I have fixed the comment in CVS.

Thanks,
Steve

Quoted 39 lines · Marcel Holtmann

Hi Albert,

Quoted 28 lines

the documentation for sdp_service_search_req() states that it outputs an sdp_list_t * of sdp_record_t * (list of sdp service records)

reading through the code, it appears that it actually returns a list of uint32_t * data structures.

line 2721:

        extract_record_handle_seq(pdata, rsp, rec_count, &scanned);

line 2523:
static void extract_record_handle_seq(char *pdu, sdp_list_t **seq, int count, int *scanned)

{
    sdp_list_t *pSeq = *seq;
    char *pdata = pdu;
    int n;

    for (n = 0; n < count; n++) {
        uint32_t *pSvcRec = (uint32_t *) malloc(sizeof(uint32_t));
        *pSvcRec = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);
        pdata += sizeof(uint32_t);
        *scanned += sizeof(uint32_t);
    }
    *seq = pSeq;
}

Quoted 8 lines

I think this will cause segfaults when you try to sdp_record_free the records in the list (it does for me).

perhaps it shold read something more like:

        sdp_record_t *pSvcRec = (sdp_record_t*) malloc(sizeof(sdp_record_t));
        pSvcRec->attrlist = pSvcRec->pattern = 0;
        pSvcRec->handle = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);

Quoted 7 lines

Steve, any thoughts on this one?

Regards

Marcel

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

it appears that the internal documentation for sdp_record_register is also incorrect.

libs/src/sdp.c:2288
*
* Returns a non-null value (a pointer) to a service
* record if successful, else -1 setting errno */

I think it should read something like:
*
* Returns zero if successful, also setting rec->handle
* else -1 setting erro
*/

You may also want to update libs/include/sdp_lib.h to indicate the behavior of sdp_record_register

Regards,
Albert

Quoted 71 lines · Stephen Crane

Hi Albert, Marcel,

The documentation is incorrect: the correct behaviour of sdp_service_search_req() is that it returns a list of service record handles --- you then use sdp_search_attr_req() to fetch attributes of interest. Alternatively, you can use sdp_search_attr_req() to perform a combined search.

I have fixed the comment in CVS.

Thanks,
Steve

Quoted 39 lines · Marcel Holtmann

Hi Albert,

Quoted 28 lines

the documentation for sdp_service_search_req() states that it outputs an sdp_list_t * of sdp_record_t * (list of sdp service records)

reading through the code, it appears that it actually returns a list of uint32_t * data structures.

line 2721:

        extract_record_handle_seq(pdata, rsp, rec_count, &scanned);

line 2523:
static void extract_record_handle_seq(char *pdu, sdp_list_t **seq, int count, int *scanned)

{
    sdp_list_t *pSeq = *seq;
    char *pdata = pdu;
    int n;

    for (n = 0; n < count; n++) {
        uint32_t *pSvcRec = (uint32_t *) malloc(sizeof(uint32_t));
        *pSvcRec = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);
        pdata += sizeof(uint32_t);
        *scanned += sizeof(uint32_t);
    }
    *seq = pSeq;
}

Quoted 8 lines

I think this will cause segfaults when you try to sdp_record_free the records in the list (it does for me).

perhaps it shold read something more like:

        sdp_record_t *pSvcRec = (sdp_record_t*) malloc(sizeof(sdp_record_t));
        pSvcRec->attrlist = pSvcRec->pattern = 0;
        pSvcRec->handle = ntohl(bt_get_unaligned((uint32_t *) pdata));
        pSeq = sdp_list_append(pSeq, pSvcRec);

Quoted 7 lines

Steve, any thoughts on this one?

Regards

Marcel

-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel