2010-05-14 03:13:30

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: [PATCH 1/1] Fixes sdp_get_supp_feat function

In case of string data items, value is a pointer by itself.
---
lib/sdp.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index fb8ccdc..c75a000 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -4801,10 +4801,15 @@ int sdp_get_supp_feat(const sdp_record_t *rec, sdp_list_t **seqp)
subseq = NULL;
for (dd = d->val.dataseq; dd; dd = dd->next) {
sdp_data_t *data;
+ void *val;
if (dd->dtd != SDP_UINT8 && dd->dtd != SDP_UINT16 &&
dd->dtd != SDP_TEXT_STR8)
goto fail;
- data = sdp_data_alloc(dd->dtd, &dd->val);
+ if (dd->dtd == SDP_TEXT_STR8)
+ val = dd->val.str;
+ else
+ val = &dd->val;
+ data = sdp_data_alloc(dd->dtd, val);
if (data)
subseq = sdp_list_append(subseq, data);
}
--
1.7.0.4



Subject: Re: [PATCH 1/1] Fixes sdp_get_supp_feat function

Hi,

El Friday 14 May 2010 14:58:00 Elvis Pf?tzenreuter escribi?:
>
> On 14/05/2010, at 04:02, Jos? Antonio Santos Cadenas wrote:
>
> > Hi Elvis,
> >
> > El Friday 14 May 2010 05:13:30 Elvis Pf?tzenreuter escribi?:
> >> In case of string data items, value is a pointer by itself.
> >> ---
> >> lib/sdp.c | 7 ++++++-
> >> 1 files changed, 6 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/lib/sdp.c b/lib/sdp.c
> >> index fb8ccdc..c75a000 100644
> >> --- a/lib/sdp.c
> >> +++ b/lib/sdp.c
> >> @@ -4801,10 +4801,15 @@ int sdp_get_supp_feat(const sdp_record_t *rec,
> >> sdp_list_t **seqp) subseq = NULL;
> >> for (dd = d->val.dataseq; dd; dd = dd->next) {
> >> sdp_data_t *data;
> >> + void *val;
> >> if (dd->dtd != SDP_UINT8 && dd->dtd != SDP_UINT16 &&
> >> dd->dtd != SDP_TEXT_STR8)
> >> goto fail;
> >> - data = sdp_data_alloc(dd->dtd, &dd->val);
> >> + if (dd->dtd == SDP_TEXT_STR8)
> >> + val = dd->val.str;
> >> + else
> >> + val = &dd->val;
> >> + data = sdp_data_alloc(dd->dtd, val);
> >
> > No only strings are pointers, if you see this patch:
> > http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=1d1154156df28660e41031df5c3f1ffe91c01aae
> >
> > that I sent few days ago for fixing the set function, also sequences are
> > pointers and should be treated in a different way. Also strings can be formed
> > by other types not only SDP_TEXT_STR8. I think a switch will fix this better.
>
> That would be nice for completeness, but the if .. goto fail just before the patch guarantees that data type will be UINT8, UINT16 or an STR8. (Am I missing something here?)

Of course, the code seems to work fine. I just suggested to modify it at once, because a
"bug"/non completeness is still there and another patch will be required. It's a little
bit more of work and the code will work in all the cases.



2010-05-14 12:58:00

by Elvis Pfutzenreuter

[permalink] [raw]
Subject: Re: [PATCH 1/1] Fixes sdp_get_supp_feat function



On 14/05/2010, at 04:02, Jos? Antonio Santos Cadenas wrote:

> Hi Elvis,
>
> El Friday 14 May 2010 05:13:30 Elvis Pf?tzenreuter escribi?:
>> In case of string data items, value is a pointer by itself.
>> ---
>> lib/sdp.c | 7 ++++++-
>> 1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/lib/sdp.c b/lib/sdp.c
>> index fb8ccdc..c75a000 100644
>> --- a/lib/sdp.c
>> +++ b/lib/sdp.c
>> @@ -4801,10 +4801,15 @@ int sdp_get_supp_feat(const sdp_record_t *rec,
>> sdp_list_t **seqp) subseq = NULL;
>> for (dd = d->val.dataseq; dd; dd = dd->next) {
>> sdp_data_t *data;
>> + void *val;
>> if (dd->dtd != SDP_UINT8 && dd->dtd != SDP_UINT16 &&
>> dd->dtd != SDP_TEXT_STR8)
>> goto fail;
>> - data = sdp_data_alloc(dd->dtd, &dd->val);
>> + if (dd->dtd == SDP_TEXT_STR8)
>> + val = dd->val.str;
>> + else
>> + val = &dd->val;
>> + data = sdp_data_alloc(dd->dtd, val);
>
> No only strings are pointers, if you see this patch:
> http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=1d1154156df28660e41031df5c3f1ffe91c01aae
>
> that I sent few days ago for fixing the set function, also sequences are
> pointers and should be treated in a different way. Also strings can be formed
> by other types not only SDP_TEXT_STR8. I think a switch will fix this better.

That would be nice for completeness, but the if .. goto fail just before the patch guarantees that data type will be UINT8, UINT16 or an STR8. (Am I missing something here?)

Subject: Re: [PATCH 1/1] Fixes sdp_get_supp_feat function

Hi Elvis,

El Friday 14 May 2010 05:13:30 Elvis Pf?tzenreuter escribi?:
> In case of string data items, value is a pointer by itself.
> ---
> lib/sdp.c | 7 ++++++-
> 1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/lib/sdp.c b/lib/sdp.c
> index fb8ccdc..c75a000 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -4801,10 +4801,15 @@ int sdp_get_supp_feat(const sdp_record_t *rec,
> sdp_list_t **seqp) subseq = NULL;
> for (dd = d->val.dataseq; dd; dd = dd->next) {
> sdp_data_t *data;
> + void *val;
> if (dd->dtd != SDP_UINT8 && dd->dtd != SDP_UINT16 &&
> dd->dtd != SDP_TEXT_STR8)
> goto fail;
> - data = sdp_data_alloc(dd->dtd, &dd->val);
> + if (dd->dtd == SDP_TEXT_STR8)
> + val = dd->val.str;
> + else
> + val = &dd->val;
> + data = sdp_data_alloc(dd->dtd, val);

No only strings are pointers, if you see this patch:
http://git.kernel.org/?p=bluetooth/bluez.git;a=commit;h=1d1154156df28660e41031df5c3f1ffe91c01aae

that I sent few days ago for fixing the set function, also sequences are
pointers and should be treated in a different way. Also strings can be formed
by other types not only SDP_TEXT_STR8. I think a switch will fix this better.

> if (data)
> subseq = sdp_list_append(subseq, data);
> }

Regards.