2012-04-10 14:47:39

by Slawomir Bochenski

Subject: [PATCH obexd v2 1/3] MAP: Mark filter strings const

This structure is used for giving filters as input argument to
messages_get_messages_listing(). There is no need for the string members
to be modifiable in this context. And making them const plays well with
map_ap_get_string(), from which the values of these members are going to
be retrieved.
---
v2: Turn on verbose mode in commit messages

plugins/messages.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/plugins/messages.h b/plugins/messages.h
index 00a040c..2cdd92f 100644
--- a/plugins/messages.h
+++ b/plugins/messages.h
@@ -111,11 +111,11 @@ struct messages_event {
struct messages_filter {
uint32_t parameter_mask;
uint8_t type;
- char *period_begin;
- char *period_end;
+ const char *period_begin;
+ const char *period_end;
uint8_t read_status;
- char *recipient;
- char *originator;
+ const char *recipient;
+ const char *originator;
uint8_t priority;
};

--
1.7.5.1

2012-04-12 11:40:17

by Johan Hedberg

Subject: Re: [PATCH obexd v2 1/3] MAP: Mark filter strings const

Hi Slawek,

On Wed, Apr 11, 2012, Slawomir Bochenski wrote:
> On Wed, Apr 11, 2012 at 3:19 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
> > Hi Slawomir,
> >
> > On Tue, Apr 10, 2012 at 5:47 PM, Slawomir Bochenski <[email protected]> wrote:
> >> This structure is used for giving filters as input argument to
> >> messages_get_messages_listing(). There is no need for the string members
> >> to be modifiable in this context. And making them const plays well with
> >> map_ap_get_string(), from which the values of these members are going to
> >> be retrieved.
> >> ---
> >> v2: Turn on verbose mode in commit messages
> >>
> >> ?plugins/messages.h | ? ?8 ++++----
> >> ?1 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/plugins/messages.h b/plugins/messages.h
> >> index 00a040c..2cdd92f 100644
> >> --- a/plugins/messages.h
> >> +++ b/plugins/messages.h
> >> @@ -111,11 +111,11 @@ struct messages_event {
> >> ?struct messages_filter {
> >> ? ? ? ?uint32_t parameter_mask;
> >> ? ? ? ?uint8_t type;
> >> - ? ? ? char *period_begin;
> >> - ? ? ? char *period_end;
> >> + ? ? ? const char *period_begin;
> >> + ? ? ? const char *period_end;
> >> ? ? ? ?uint8_t read_status;
> >> - ? ? ? char *recipient;
> >> - ? ? ? char *originator;
> >> + ? ? ? const char *recipient;
> >> + ? ? ? const char *originator;
> >> ? ? ? ?uint8_t priority;
> >> ?};
> >>
> >> --
> >> 1.7.5.1
> >
> > Don't you need to free them at some point? If it is just a reference
> > then it is probably fine, but if you have to free them then it might
> > be better this way.
>
> Just reference to strings stored in another data structure, which
> takes care of freeing them when request is finished.

That should be mentioned in the commit message (and if it had been from
the start all this unnecessary discussion could have been avoided).

Johan

2012-04-11 14:04:02

by Slawomir Bochenski

Subject: Re: [PATCH obexd v2 1/3] MAP: Mark filter strings const

On Wed, Apr 11, 2012 at 3:19 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Slawomir,
>
> On Tue, Apr 10, 2012 at 5:47 PM, Slawomir Bochenski <[email protected]> wrote:
>> This structure is used for giving filters as input argument to
>> messages_get_messages_listing(). There is no need for the string members
>> to be modifiable in this context. And making them const plays well with
>> map_ap_get_string(), from which the values of these members are going to
>> be retrieved.
>> ---
>> v2: Turn on verbose mode in commit messages
>>
>> ?plugins/messages.h | ? ?8 ++++----
>> ?1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/plugins/messages.h b/plugins/messages.h
>> index 00a040c..2cdd92f 100644
>> --- a/plugins/messages.h
>> +++ b/plugins/messages.h
>> @@ -111,11 +111,11 @@ struct messages_event {
>> ?struct messages_filter {
>> ? ? ? ?uint32_t parameter_mask;
>> ? ? ? ?uint8_t type;
>> - ? ? ? char *period_begin;
>> - ? ? ? char *period_end;
>> + ? ? ? const char *period_begin;
>> + ? ? ? const char *period_end;
>> ? ? ? ?uint8_t read_status;
>> - ? ? ? char *recipient;
>> - ? ? ? char *originator;
>> + ? ? ? const char *recipient;
>> + ? ? ? const char *originator;
>> ? ? ? ?uint8_t priority;
>> ?};
>>
>> --
>> 1.7.5.1
>
> Don't you need to free them at some point? If it is just a reference
> then it is probably fine, but if you have to free them then it might
> be better this way.

Just reference to strings stored in another data structure, which
takes care of freeing them when request is finished.

>
> --
> Luiz Augusto von Dentz

--
Slawomir Bochenski

2012-04-11 13:19:54

by Luiz Augusto von Dentz

Subject: Re: [PATCH obexd v2 1/3] MAP: Mark filter strings const

Hi Slawomir,

On Tue, Apr 10, 2012 at 5:47 PM, Slawomir Bochenski <[email protected]> wrote:
> This structure is used for giving filters as input argument to
> messages_get_messages_listing(). There is no need for the string members
> to be modifiable in this context. And making them const plays well with
> map_ap_get_string(), from which the values of these members are going to
> be retrieved.
> ---
> v2: Turn on verbose mode in commit messages
>
> ?plugins/messages.h | ? ?8 ++++----
> ?1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/plugins/messages.h b/plugins/messages.h
> index 00a040c..2cdd92f 100644
> --- a/plugins/messages.h
> +++ b/plugins/messages.h
> @@ -111,11 +111,11 @@ struct messages_event {
> ?struct messages_filter {
> ? ? ? ?uint32_t parameter_mask;
> ? ? ? ?uint8_t type;
> - ? ? ? char *period_begin;
> - ? ? ? char *period_end;
> + ? ? ? const char *period_begin;
> + ? ? ? const char *period_end;
> ? ? ? ?uint8_t read_status;
> - ? ? ? char *recipient;
> - ? ? ? char *originator;
> + ? ? ? const char *recipient;
> + ? ? ? const char *originator;
> ? ? ? ?uint8_t priority;
> ?};
>
> --
> 1.7.5.1

Don't you need to free them at some point? If it is just a reference
then it is probably fine, but if you have to free them then it might
be better this way.


--
Luiz Augusto von Dentz

2012-04-10 14:47:41

by Slawomir Bochenski

Subject: [PATCH obexd v2 3/3] MAP: Output parameters for GetMessagesListing

This sets the parameters returned from MAP backend for
GetMessagesListing to be used in application parameters header sent in
response.
---
plugins/mas.c | 17 +++++++++++++++++
1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/plugins/mas.c b/plugins/mas.c
index 76dc8e9..576c206 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -282,12 +282,22 @@ static void get_messages_listing_cb(void *session, int err, uint16_t size,
void *user_data)
{
struct mas_session *mas = user_data;
+ uint16_t max = 1024;

if (err < 0 && err != -EAGAIN) {
obex_object_set_io_flags(mas, G_IO_ERR, err);
return;
}

+ map_ap_get_u16(mas->inparams, MAP_AP_MAXLISTCOUNT, &max);
+
+ if (max == 0) {
+ if (!entry)
+ mas->finished = TRUE;
+
+ goto proceed;
+ }
+
if (!mas->nth_call) {
g_string_append(mas->buffer, ML_BODY_BEGIN);
mas->nth_call = TRUE;
@@ -379,6 +389,13 @@ static void get_messages_listing_cb(void *session, int err, uint16_t size,
g_string_append(mas->buffer, "/>\n");

proceed:
+ if (!entry) {
+ map_ap_set_u16(mas->outparams, MAP_AP_MESSAGESLISTINGSIZE,
+ size);
+ map_ap_set_u8(mas->outparams, MAP_AP_NEWMESSAGE,
+ newmsg ? 1 : 0);
+ }
+
if (err != -EAGAIN)
obex_object_set_io_flags(mas, G_IO_IN, 0);
}
--
1.7.5.1

2012-04-10 14:47:40

by Slawomir Bochenski

Subject: [PATCH obexd v2 2/3] MAP: Input parameters for GetMessagesListing

This adds support for input application parameters header given in
GetMessagesListing request.
---
plugins/mas.c | 27 +++++++++++++++++++++++++--
1 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/plugins/mas.c b/plugins/mas.c
index 97a37e4..76dc8e9 100644
--- a/plugins/mas.c
+++ b/plugins/mas.c
@@ -531,6 +531,9 @@ static void *msg_listing_open(const char *name, int oflag, mode_t mode,
{
struct mas_session *mas = driver_data;
struct messages_filter filter = { 0, };
+ /* 1024 is the default when there was no MaxListCount sent */
+ uint16_t max = 1024;
+ uint16_t offset = 0;

DBG("");

@@ -539,8 +542,28 @@ static void *msg_listing_open(const char *name, int oflag, mode_t mode,
return NULL;
}

- *err = messages_get_messages_listing(mas->backend_data, name, 0xffff, 0,
- &filter,
+ map_ap_get_u16(mas->inparams, MAP_AP_MAXLISTCOUNT, &max);
+ map_ap_get_u16(mas->inparams, MAP_AP_STARTOFFSET, &offset);
+
+ map_ap_get_u32(mas->inparams, MAP_AP_PARAMETERMASK,
+ &filter.parameter_mask);
+ map_ap_get_u8(mas->inparams, MAP_AP_FILTERMESSAGETYPE,
+ &filter.type);
+ filter.period_begin = map_ap_get_string(mas->inparams,
+ MAP_AP_FILTERPERIODBEGIN);
+ filter.period_end = map_ap_get_string(mas->inparams,
+ MAP_AP_FILTERPERIODEND);
+ map_ap_get_u8(mas->inparams, MAP_AP_FILTERREADSTATUS,
+ &filter.read_status);
+ filter.recipient = map_ap_get_string(mas->inparams,
+ MAP_AP_FILTERRECIPIENT);
+ filter.originator = map_ap_get_string(mas->inparams,
+ MAP_AP_FILTERORIGINATOR);
+ map_ap_get_u8(mas->inparams, MAP_AP_FILTERPRIORITY,
+ &filter.priority);
+
+ *err = messages_get_messages_listing(mas->backend_data, name, max,
+ offset, &filter,
get_messages_listing_cb, mas);

mas->buffer = g_string_new("");
--
1.7.5.1