On Wed, Jul 20, 2011 at 2:23 PM, Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
> Hi,
>
> On Wed, Jul 20, 2011 at 2:14 PM, Slawomir Bochenski <lkslawek@xxxxxxxxx> wrote:
>> On Wed, Jul 20, 2011 at 12:47 PM, Luiz Augusto von Dentz
>> <luiz.dentz@xxxxxxxxx> wrote:
>>> Hi,
>>>
>>> On Wed, Jul 20, 2011 at 1:08 PM, Slawomir Bochenski <lkslawek@xxxxxxxxx> wrote:
>>>> Hi,
>>>>
>>>> On Wed, Jul 20, 2011 at 11:50 AM, Luiz Augusto von Dentz
>>>> <luiz.dentz@xxxxxxxxx> wrote:
>>>>> Hi,
>>>>>
>>>>> On Tue, Jul 19, 2011 at 3:06 PM, Slawomir Bochenski <lkslawek@xxxxxxxxx> wrote:
>>>>>> ---
>>>>>> plugins/mas.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++----------
>>>>>> 1 files changed, 64 insertions(+), 14 deletions(-)
>>>>>>
>>>>>> diff --git a/plugins/mas.c b/plugins/mas.c
>>>>>> index 0ef8c81..e88a0f0 100644
>>>>>> --- a/plugins/mas.c
>>>>>> +++ b/plugins/mas.c
>>>>>> @@ -27,6 +27,9 @@
>>>>>>
>>>>>> #include <errno.h>
>>>>>> #include <glib.h>
>>>>>> +#include <sys/types.h>
>>>>>> +#include <sys/stat.h>
>>>>>> +#include <fcntl.h>
>>>>>> #include <openobex/obex.h>
>>>>>>
>>>>>> #include "plugin.h"
>>>>>> @@ -86,9 +89,19 @@
>>>>>> </attribute> \
>>>>>> </record>"
>>>>>>
>>>>>> +#define EVENT_TYPE "x-bt/MAP-event-report"
>>>>>> +#define MESSAGE_TYPE "x-bt/message"
>>>>>> +#define FOLDER_LISTING_TYPE "x-obex/folder-listing"
>>>>>> +#define MESSAGES_LISTING_TYPE "x-bt/MAP-msg-listing"
>>>>>> +#define NOTIFICATION_TYPE "x-bt/MAP-NotificationRegistration"
>>>>>> +#define STATUS_TYPE "x-bt/messageStatus"
>>>>>> +#define UPDATE_TYPE "x-bt/MAP-messageUpdate"
>>>>>> +
>>>>>> struct mas_session {
>>>>>> struct mas_request *request;
>>>>>> void *backend_data;
>>>>>> + const char *name;
>>>>>> + const char *type;
>>>>>> };
>>>>>>
>>>>>> static const uint8_t MAS_TARGET[TARGET_SIZE] = {
>>>>>> @@ -137,17 +150,14 @@ static void mas_disconnect(struct obex_session *os, void *user_data)
>>>>>> static int mas_get(struct obex_session *os, obex_object_t *obj, void *user_data)
>>>>>> {
>>>>>> struct mas_session *mas = user_data;
>>>>>> - const char *type = obex_get_type(os);
>>>>>> - const char *name = obex_get_name(os);
>>>>>> int ret;
>>>>>>
>>>>>> - DBG("GET: name %s type %s mas %p",
>>>>>> - name, type, mas);
>>>>>> + mas->name = obex_get_name(os);
>>>>>> + mas->type = obex_get_type(os);
>>>>>>
>>>>>> - if (type == NULL)
>>>>>> - return -EBADR;
>>>>>> + DBG("GET: name %s type %s mas %p", mas->name, mas->type, mas);
>>>>>>
>>>>>> - ret = obex_get_stream_start(os, name);
>>>>>> + ret = obex_get_stream_start(os, mas->name);
>>>>>> if (ret < 0)
>>>>>> goto failed;
>>>>>>
>>>>>> @@ -160,16 +170,14 @@ failed:
>>>>>> static int mas_put(struct obex_session *os, obex_object_t *obj, void *user_data)
>>>>>> {
>>>>>> struct mas_session *mas = user_data;
>>>>>> - const char *type = obex_get_type(os);
>>>>>> - const char *name = obex_get_name(os);
>>>>>> int ret;
>>>>>>
>>>>>> - DBG("PUT: name %s type %s mas %p", name, type, mas);
>>>>>> + mas->name = obex_get_name(os);
>>>>>> + mas->type = obex_get_type(os);
>>>>>>
>>>>>> - if (type == NULL)
>>>>>> - return -EBADR;
>>>>>> + DBG("PUT: name %s type %s mas %p", mas->name, mas->type, mas);
>>>>>>
>>>>>> - ret = obex_put_stream_start(os, name);
>>>>>> + ret = obex_put_stream_start(os, mas->name);
>>>>>> if (ret < 0)
>>>>>> goto failed;
>>>>>>
>>>>>> @@ -179,6 +187,38 @@ failed:
>>>>>> return ret;
>>>>>> }
>>>>>
>>>>> I would suggest adding a mimetype driver for each type so you don't
>>>>> have to do the check inside the service driver and it is also
>>>>> consistent with what other plugins do when they support multiple
>>>>> types.
>>>>>
>>>> I would not. That's the whole point. Functions for the mime driver
>>>> would do exactly the same.
>>>
>>> There we go again, so your point is that you don't want to be
>>> consistent with other plugins and you like to see duplicated code
>>> checking for matching types the very same way the core does for
>>> mimetype drivers? Or do you mean the spec got this wrong and there is
>>> no use of types because all of them do exactly the same?
>>
>> This was discussed before. More than half of "types" are not really
>> mime types at all (there's no even body involved in request). Type is
>> used in MAP merely as a function selector (this _is_ called a function
>> in MAP). All specific handling of request is done in callback from MAP
>> API. The only thing that each and every mime driver would do is to
>> read buffer prepared by a callback. I've specifically chosen to use
>> the already present feature of universal target-specific mime driver
>> in order to avoid registering 6 mime drivers with the same handlers,
>> or worse - 6 times identical functions. The approach is minimalistic
>> and pretty. Implementation is easy to understand in terms of MAP
>> specification instead of putting it into some obexd specific
>> abstraction.
>
> What does a function selection mean? Doesn't it means they do
> something different? Why you are saying you will have 6 driver with
> the same handlers if they map to different functions? I would
> understand if you said you have done this way and it works just fine,
Yes, I've done it. Works just fine.
> but arguing that your approach is minimalistic and pretty? What about
> consistency, reliability and maintainability? The abstraction is there
> no matter how hard you try to make it go away, it is the interface to
> which the core daemon talks to backends, it doesn't really matter what
> you put in between. Your minimalistic approach just confuses the core
> daemon making it believe there is no driver for a specific mimetype
> selecting its default fallback for the target.
Nothing confusing here. Just the normal driver selection.
>
>>>Sorry but I
>>> don't think this has any valid technical point, in fact the mimetype
>>> and service separation does help plugins to not mix object and session
>>> handling.
>>
>> There can be only one active request per session. No mixing problems there.
>
> Partially correct, basically you can only handle one request per
> session, period. Note, it doesn't break OBEX or MAP, but if we already
> have infrastructure in place to handle this properly why not do it?
> Why not doing it right since the beginning? Shall our answer be
> minimalism and prettiness?
"Right" is quite a subjective term here.
>
>>>>>> +static int start_get(struct mas_session *mas)
>>>>>> +{
>>>>>> + /* NOTE: type is case-insensitive! */
>>>>>> + if (g_ascii_strcasecmp(mas->type, FOLDER_LISTING_TYPE) == 0)
>>>>>> + return -EINVAL;
>>>>>> + else if (g_ascii_strcasecmp(mas->type, MESSAGES_LISTING_TYPE) == 0)
>>>>>> + return -EINVAL;
>>>>>> + else if (g_ascii_strcasecmp(mas->type, MESSAGE_TYPE) == 0)
>>>>>> + return -EINVAL;
>>>>>> + else {
>>>>>> + DBG("Incorrect type for get: %s", mas->type);
>>>>>> + return -EBADR;
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> +static int start_put(struct mas_session *mas)
>>>>>> +{
>>>>>> + /* NOTE: type is case-insensitive! */
>>>>>> + if (g_ascii_strcasecmp(mas->type, NOTIFICATION_TYPE) == 0)
>>>>>> + return -EINVAL;
>>>>>> + else if (g_ascii_strcasecmp(mas->type, STATUS_TYPE) == 0)
>>>>>> + return -EINVAL;
>>>>>> + else if (g_ascii_strcasecmp(mas->type, MESSAGE_TYPE) == 0)
>>>>>> + return -EINVAL;
>>>>>> + else if (g_ascii_strcasecmp(mas->type, UPDATE_TYPE) == 0)
>>>>>> + return -EINVAL;
>>>>>> + else {
>>>>>> + DBG("Incorrect type for put: %s", mas->type);
>>>>>> + return -EBADR;
>>>>>> + }
>>>>>> +}
>>>>>> +
>>>>>> static int mas_setpath(struct obex_session *os, obex_object_t *obj,
>>>>>> void *user_data)
>>>>>> {
>>>>>> @@ -210,7 +250,17 @@ static void *any_open(const char *name, int oflag, mode_t mode,
>>>>>>
>>>>>> DBG("");
>>>>>>
>>>>>> - *err = 0;
>>>>>> + if ((oflag & O_RDONLY) == O_RDONLY) {
>>>>>> + *err = start_get(mas);
>>>>>> + } else if ((oflag & O_WRONLY) == O_WRONLY) {
>>>>>> + *err = start_put(mas);
>>>>>> + } else {
>>>>>> + DBG("Invalid open flag!");
>>>>>> + *err = -EIO;
>>>>>> + }
>>>>>> +
>>>>>> + if (*err)
>>>>>> + return NULL;
>>>>>>
>>>>>> return mas;
>>>>>> }
>>>>>> --
>>>>>> 1.7.4.1
>>>>>
>>>>> --
>>>>> Luiz Augusto von Dentz
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Slawomir Bochenski
>>>>
>>>
>>>
>>>
>>> --
>>> Luiz Augusto von Dentz
>>>
>>
>>
>>
>> --
>> Slawomir Bochenski
>>
>
>
>
> --
> Luiz Augusto von Dentz
>
--
Slawomir Bochenski
--
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
