2005-02-25 19:24:36

by Albert Huang

[permalink] [raw]
Subject: [Bluez-devel] 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);

Regards,
-albert


-------------------------------------------------------
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


2005-02-28 12:20:38

by Stephen Crane

[permalink] [raw]
Subject: Re: [Bluez-devel] confusion about libs/sdp.c - sdp_service_search_req()

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

On Mon, 2005-02-28 at 09:34 +0100, Marcel Holtmann wrote:
> Hi Albert,
>
> > 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);
>
> Steve, any thoughts on this one?
>
> Regards
>
> Marcel
>
>

2005-02-28 08:34:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] confusion about libs/sdp.c - sdp_service_search_req()

Hi Albert,

> 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);

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

2005-03-02 08:48:53

by Albert Huang

[permalink] [raw]
Subject: Re: [Bluez-devel] confusion about libs/sdp.c - sdp_service_search_req()

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

On Mon, 28 Feb 2005 12:20:38 +0000, Stephen Crane
<[email protected]> wrote:
> 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
>
> On Mon, 2005-02-28 at 09:34 +0100, Marcel Holtmann wrote:
> > Hi Albert,
> >
> > > 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);
> >
> > 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
>


-------------------------------------------------------
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

2005-03-02 10:23:28

by Stephen Crane

[permalink] [raw]
Subject: Re: [Bluez-devel] confusion about libs/sdp.c - sdp_service_search_req()

Hi Albert,
I have made the changes as you suggest. In future, it would probably be
just as easy for you to send patches, right?

Thanks,
Steve

On Wed, 2005-03-02 at 03:48 -0500, Albert Huang wrote:
> 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
>
> On Mon, 28 Feb 2005 12:20:38 +0000, Stephen Crane
> <[email protected]> wrote:
> > 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
> >
> > On Mon, 2005-02-28 at 09:34 +0100, Marcel Holtmann wrote:
> > > Hi Albert,
> > >
> > > > 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);
> > >
> > > 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
> >


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part