On Thu, 07 Jul 2011 08:09:01 +0200, Carsten Haitzler
<ras...@rasterman.com> wrote:
On Wed, 06 Jul 2011 15:51:41 +0200 "Libor Zoubek" <lzou...@jezzovo.net>
said:
thanks for your reply,
comments inline
On Tue, 05 Jul 2011 08:40:53 +0200, Carsten Haitzler
<ras...@rasterman.com> wrote:
> On Wed, 29 Jun 2011 16:01:06 +0200 "Libor Zoubek"
<lzou...@jezzovo.net>
> said:
>
>> Hi, devs
>>
>> I've reported a bug http://trac.enlightenment.org/e/ticket/775
>> Please review/apply attached patch that fixes above bug.
>>
>> Thanks
>>
>> cheers,
>>
>> Libor Zoubek
>>
>> P.S. I am a happy e-user and this is my 1st attempt to contribute
>
> cool!... patch kind of good. it moves forward to a new version (1.2)
> BUT...
> your patch makes edbus daemon still say it does 0.9 but the spec you
are
> making
> it move to is 1.2 (more strings). so E_NOTIFICATION_DAEMON_VERSION and
> E_NOTIFICATION_DAEMON_SUPPORTS_SPEC_VERSION should say "1.2"...
you are right, I wasn' sure. But as far as I am reading throught
differences between 0.9 and 1.2, it seems like only getServerInformation
signature changed. There are also defined new capabilities, that server
can support.
> also e_notify_unmarshal_get_server_information_return doesnt seem to
be
> backwards compatible - ie it doesnt handle "sss" vs "ssss" signature
> (extra
> version string on the end). we'd be pretty bad breaking compat to 0.9.
> so we
> should do "if "sss" > handle 0.9, else if "ssss" > handle 1.2" like
> logic there.
>
> :)
>
I am not sure how to do this and it if is even possible. Scenario is:
client calls getServerInformation via dbus, server responses something
("sss" or "ssss"), but client has not capability to say "I am able to
talk
to server, which supports spec version X". thatswhy I don't know how
should server recognize if returns "sss" or "ssss"
I've been digging into gnome-notification daemon implementation and did
not found anything that makes it compatible with < 0.9 spec,
well you still #define 0.9 as version but do 1.2 - so thats kind of
misleading.
but the code in the client handler you changed SPECIFICALLY does:
if (!dbus_message_has_signature(msg, "ssss")) return NULL;
that means if server is the old 0.9 with "sss" it will fail as a client
to get
server info. why not just do (pseudo code)
if (sss) { get name, vendor, version) }
else if (ssss) { get name, vendor, version, spec_version }
else return NULL;
sure - u can merge sss and ssss case as there is just 1 added field. but
why
make it break with older servers?
I got your point, finally :-) I haven't thought about e_dbus notification
to act as client (which would report notifications to other but e server).
Attached patch v2
Libor
see
http://git.gnome.org/browse/notification-daemon/tree/src/daemon/daemon.c?h=0.5
line 1731
or http://git.gnome.org/browse/notification-daemon/tree/src/daemon.c
line
288
Index: src/lib/notification/E_Notify.h
===================================================================
--- src/lib/notification/E_Notify.h (revision 61106)
+++ src/lib/notification/E_Notify.h (working copy)
@@ -76,6 +76,7 @@
const char *name;
const char *vendor;
const char *version;
+ const char *spec_version;
};
/* signals */
Index: src/lib/notification/E_Notification_Daemon.h
===================================================================
--- src/lib/notification/E_Notification_Daemon.h (revision 61106)
+++ src/lib/notification/E_Notification_Daemon.h (working copy)
@@ -2,7 +2,7 @@
#define E_NOTIFICATION_DAEMON_H
#define E_NOTIFICATION_DAEMON_VERSION "0.9"
-
+#define E_NOTIFICATION_DAEMON_SUPPORTS_SPEC_VERSION "1.2"
#include <E_Notify.h>
#ifdef EAPI
Index: src/lib/notification/daemon.c
===================================================================
--- src/lib/notification/daemon.c (revision 61106)
+++ src/lib/notification/daemon.c (working copy)
@@ -68,7 +68,7 @@
daemon = e_dbus_object_data_get(obj);
- return e_notify_marshal_get_server_information_return(message, daemon->name, daemon->vendor, E_NOTIFICATION_DAEMON_VERSION);
+ return e_notify_marshal_get_server_information_return(message, daemon->name, daemon->vendor, E_NOTIFICATION_DAEMON_VERSION, E_NOTIFICATION_DAEMON_SUPPORTS_SPEC_VERSION);
}
@@ -126,7 +126,7 @@
e_dbus_interface_method_add(daemon->iface, "GetCapabilities", "", "as", method_get_capabilities);
e_dbus_interface_method_add(daemon->iface, "Notify", "susssasa{sv}i", "u", method_notify);
e_dbus_interface_method_add(daemon->iface, "CloseNotification", "u", "u", method_close_notification);
- e_dbus_interface_method_add(daemon->iface, "GetServerInformation", "", "sss", method_get_server_information);
+ e_dbus_interface_method_add(daemon->iface, "GetServerInformation", "", "ssss", method_get_server_information);
return daemon;
}
Index: src/lib/notification/marshal.c
===================================================================
--- src/lib/notification/marshal.c (revision 61106)
+++ src/lib/notification/marshal.c (working copy)
@@ -178,11 +178,11 @@
}
DBusMessage *
-e_notify_marshal_get_server_information_return(DBusMessage *method_call, const char *name, const char *vendor, const char *version)
+e_notify_marshal_get_server_information_return(DBusMessage *method_call, const char *name, const char *vendor, const char *version, const char *spec_version)
{
DBusMessage *msg;
msg = dbus_message_new_method_return(method_call);
- dbus_message_append_args(msg, DBUS_TYPE_STRING, &name, DBUS_TYPE_STRING, &vendor, DBUS_TYPE_STRING, &version, DBUS_TYPE_INVALID);
+ dbus_message_append_args(msg, DBUS_TYPE_STRING, &name, DBUS_TYPE_STRING, &vendor, DBUS_TYPE_STRING, &version, DBUS_TYPE_STRING, &spec_version, DBUS_TYPE_INVALID);
return msg;
}
@@ -190,21 +190,37 @@
e_notify_unmarshal_get_server_information_return(DBusMessage *msg, DBusError *err)
{
E_Notification_Return_Get_Server_Information *info;
- if (!dbus_message_has_signature(msg, "sss")) return NULL;
-
- info = calloc(1, sizeof(E_Notification_Return_Get_Server_Information));
- dbus_message_get_args(msg, err,
- DBUS_TYPE_STRING, &(info->name),
- DBUS_TYPE_STRING, &(info->vendor),
- DBUS_TYPE_STRING, &(info->version),
- DBUS_TYPE_INVALID
- );
+ if (dbus_message_has_signature(msg, "ssss"))
+ {
+ info = calloc(1, sizeof(E_Notification_Return_Get_Server_Information));
+ dbus_message_get_args(msg, err,
+ DBUS_TYPE_STRING, &(info->name),
+ DBUS_TYPE_STRING, &(info->vendor),
+ DBUS_TYPE_STRING, &(info->version),
+ DBUS_TYPE_STRING, &(info->spec_version),
+ DBUS_TYPE_INVALID
+ );
+ }
+ else if (dbus_message_has_signature(msg,"sss"))
+ {
+ // stay combatible with servers <= 0.9 spec
+ info = calloc(1, sizeof(E_Notification_Return_Get_Server_Information));
+ dbus_message_get_args(msg, err,
+ DBUS_TYPE_STRING, &(info->name),
+ DBUS_TYPE_STRING, &(info->vendor),
+ DBUS_TYPE_STRING, &(info->version),
+ DBUS_TYPE_INVALID
+ );
+ }
+ else
+ {
+ return NULL;
+ }
if (dbus_error_is_set(err))
{
free(info);
return NULL;
}
-
return info;
}
Index: src/lib/notification/e_notify_private.h
===================================================================
--- src/lib/notification/e_notify_private.h (revision 61106)
+++ src/lib/notification/e_notify_private.h (working copy)
@@ -23,7 +23,7 @@
void * e_notify_unmarshal_get_capabilities_return(DBusMessage *msg, DBusError *err);
void e_notify_free_get_capabilities_return(void *data);
DBusMessage * e_notify_marshal_get_server_information();
-DBusMessage * e_notify_marshal_get_server_information_return(DBusMessage *method_call, const char *name, const char *vendor, const char *version);
+DBusMessage * e_notify_marshal_get_server_information_return(DBusMessage *method_call, const char *name, const char *vendor, const char *version, const char *spec_version);
void * e_notify_unmarshal_get_server_information_return(DBusMessage *msg, DBusError *err);
void e_notify_free_get_server_information_return(void *data);
DBusMessage * e_notify_marshal_close_notification(dbus_uint32_t id);
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2d-c2
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
