Hi guys, I'm currently working on Ruby bindings for the service method stuff, and I noticed something's strange.
See eg http://repo.or.cz/w/xmms2-tutorial.git?a=blob;f=c/service_client/recursion.c;h=50d4af56a3e836dfb07ffec31707f5ef5f3195c6;hb=HEAD as an example of a service method. atm, service methods get passed an xmmsc_result_t*, that is only used to check for errors (xmmsc_result_iserror, xmmsc_result_get_error) and to get the arguments to the service method (xmmsc_result_get_dict_entry_...). It's also used to return a value from the service method, as it is passed to xmmsc_service_return(). I believe it is *very* wrong to pass an xmmsc_result_t* there! In libxmmsclient, results are used as handles to the xmmsc_blah functions, all of trigger some action in the server and return some data. Results can be handled sync or async, that's why xmmsc_result_notifier_set is defined for *all* of them. This just doesn't apply to the argument to the service method. What we get there isn't a common result, but some strange bastard ;) It makes no sense at all to call xmmsc_result_notifier_set on it, for example, or xmmsc_result_wait, or xmmsc_result_get_value. It also complicates the code in the Ruby bindings for me (and maybe other binding authors? :) Do you agree with me so far? So my point is that we shouldn't pass a common result to the service method callback. The quickest solution that I can think of is implemented in the attached patch. With that patch, we still pass a result, but the client author doesn't know about it ;D Comment, please! The second nitpick I found is how a value is returned from a service method. atm, you do this: result2 = xmmsc_service_return (conn, res, method); xmmsc_result_unref (result2); Can't we just make the service method callbacks return eg SERVICE_METHOD_HANDLED, which results in the above two calls? If there should be no return value, they could return some other nice value. Maybe we could get away with a boolean, too. Seems the current code is more complicated than it could be. Anyway, let me know what you think. Regards, Tilman -- A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing on usenet and in e-mail?
diff --git a/src/clients/lib/ruby/rb_xmmsclient.c
b/src/clients/lib/ruby/rb_xmmsclient.c
index 8d65f7a..b470eb2 100644
--- a/src/clients/lib/ruby/rb_xmmsclient.c
+++ b/src/clients/lib/ruby/rb_xmmsclient.c
@@ -23,6 +23,7 @@
#include "rb_xmmsclient.h"
#include "rb_playlist.h"
#include "rb_collection.h"
+#include "rb_service_method.h"
#include "rb_result.h"
#define METHOD_HANDLER_HEADER \
@@ -1555,4 +1556,5 @@ Init_Client (VALUE mXmms)
Init_Result (mXmms);
cPlaylist = Init_Playlist (mXmms);
Init_Collection (mXmms);
+ Init_ServiceMethod (mXmms);
}
diff --git a/src/clients/lib/xmmsclient/service.c
b/src/clients/lib/xmmsclient/service.c
index 9efe092..600df06 100644
--- a/src/clients/lib/xmmsclient/service.c
+++ b/src/clients/lib/xmmsclient/service.c
@@ -301,16 +301,18 @@ xmmsc_service_request (xmmsc_connection_t *conn, const
char *service,
* All return values will be reset after this call.
*
* @param conn The connection to the server.
- * @param res The result containing the service cookie.
+ * @param data The magic cookie thing.
* @param method The #xmmsc_service_method_t structure.
*/
xmmsc_result_t *
-xmmsc_service_return (xmmsc_connection_t *conn, xmmsc_result_t *res,
+xmmsc_service_return (xmmsc_connection_t *conn,
+ xmmsc_service_notifier_data_t *data,
xmmsc_service_method_t *method)
{
xmms_ipc_msg_t *msg;
uint32_t cookie;
x_list_t *n;
+ xmmsc_result_t *res = (xmmsc_result_t *) data;
xmmsc_service_argument_t *arg;
x_check_conn (conn, NULL);
@@ -1288,7 +1290,49 @@ dispatch (xmmsc_result_t *res, void *data)
x_return_if_fail (method);
- method->func (method->conn, res, method, method->udata);
+ method->func (method->conn, (xmmsc_service_notifier_data_t *) res,
+ method, method->udata);
+}
+
+int
+xmmsc_service_notifier_data_iserror (xmmsc_service_notifier_data_t *data)
+{
+ return xmmsc_result_iserror ((xmmsc_result_t *) data);
+}
+
+const char *
+xmmsc_service_notifier_data_get_error (xmmsc_service_notifier_data *data)
+{
+ return xmmsc_result_get_error ((xmmsc_result_t *) data);
+}
+
+int
+xmmsc_service_notifier_data_get_arg_string (
+ xmmsc_service_notifier_data_t *data, const char *key, char **r)
+{
+ return xmmsc_result_get_dict_entry_string ((xmmsc_result_t *) data,
key, r);
+}
+
+int
+xmmsc_service_notifier_data_get_arg_int (
+ xmmsc_service_notifier_data_t *data, const char *key, int32_t *r)
+{
+ return xmmsc_result_get_dict_entry_int ((xmmsc_result_t *) data, key,
r);
+}
+
+int
+xmmsc_service_notifier_data_get_arg_uint (
+ xmmsc_service_notifier_data_t *data, const char *key, uint32_t *r)
+{
+ return xmmsc_result_get_dict_entry_uint ((xmmsc_result_t *) data, key,
r);
+}
+
+int
+xmmsc_service_notifier_data_get_arg_collection (
+ xmmsc_service_notifier_data_t *data, const char *key,
+ xmmsc_coll_t **coll)
+{
+ return xmmsc_result_get_dict_entry_collection ((xmmsc_result_t *) data,
key, coll);
}
/* @} */
diff --git a/src/include/xmmsclient/xmmsclient.h
b/src/include/xmmsclient/xmmsclient.h
index fd7f303..7dda254 100644
--- a/src/include/xmmsclient/xmmsclient.h
+++ b/src/include/xmmsclient/xmmsclient.h
@@ -288,7 +288,8 @@ typedef void (*xmmsc_result_notifier_t) (xmmsc_result_t
*res, void *user_data);
/*
* Service Client
*/
-typedef void (*xmmsc_service_notifier_t) (xmmsc_connection_t *conn,
xmmsc_result_t *res, xmmsc_service_method_t *method, void *user_data);
+typedef void xmmsc_service_notifier_data_t;
+typedef void (*xmmsc_service_notifier_t) (xmmsc_connection_t *conn,
xmmsc_service_notifier_data_t *data, xmmsc_service_method_t *method, void
*user_data);
xmmsc_result_t *xmmsc_service_register (xmmsc_connection_t *conn, const char
*name, const char *description, uint32_t major, uint32_t minor);
xmmsc_result_t *xmmsc_service_method_register (xmmsc_connection_t *conn, const
char *service, xmmsc_service_method_t *method);
@@ -309,6 +310,11 @@ xmmsc_result_t *xmmsc_broadcast_service_shutdown
(xmmsc_connection_t *c);
xmmsc_service_method_t *xmmsc_service_method_new (const char *name, const char
*description, xmmsc_service_notifier_t func, void *user_data);
xmmsc_service_method_t *xmmsc_service_method_new_full (const char *name, const
char *description, xmmsc_service_notifier_t func, void *user_data,
xmmsc_user_data_free_func_t free_func);
+int xmmsc_service_notifier_data_get_arg_string (xmmsc_service_notifier_data_t
*data, const char *key, char **r);
+int xmmsc_service_notifier_data_get_arg_int (xmmsc_service_notifier_data_t
*data, const char *key, int32_t *r);
+int xmmsc_service_notifier_data_get_arg_uint (xmmsc_service_notifier_data_t
*data, const char *key, uint32_t *r);
+int xmmsc_service_notifier_data_get_arg_collection
(xmmsc_service_notifier_data_t *data, const char *key, xmmsc_coll_t **coll);
+
/*
* RESULTS
*/
pgpAf9qwA7j9Z.pgp
Description: PGP signature
-- _______________________________________________ Xmms2-devel mailing list Xmms2-devel@lists.xmms.se http://lists.xmms.se/cgi-bin/mailman/listinfo/xmms2-devel
