[dbus-cplusplus-devel] API issue in request_name()

[Randell J. <rj...@wg...>]

The request_name() API has an issue:

Currently it looks like this:
void Connection::request_name(const char *name, int flags)
{
...
	dbus_bus_request_name(_pvt->conn, name, flags, e);	//we deliberately don't check return value
...
}

It explicitly ignores the return code from dbus_bus_request_name().  It
does pass the flags, such as DBUS_NAME_FLAG_DO_NOT_QUEUE,
..._REPLACE_EXISTING, etc

This is a problem, since if you get an (ignored) response such as
DBUS_REQUEST_NAME_REPLY_EXISTS, you as the caller to request_name have
no way to tell if you ended up owning the name.  This makes implementing
a singleton object problematic.  I may also be missing something about
how the API is supposed to work (perhaps related to the match set
up...), but it's not obvious.

request_name() should export the result so the application can decide if
it wants to take action.

// flags are normal dbus_bus_request_name flags, result is normal return
int Connection::request_name(const char *name, int flags)
{
	InternalError e;
        int result;

	debug_log("%s: registering bus name %s", unique_name(), name);

	result = dbus_bus_request_name(_pvt->conn, name, flags, e);

	if (e) throw Error(e);

//	this->remove_match("destination");

	if (name)
	{
		_pvt->names.push_back(name);
		std::string match = "destination='" + _pvt->names.back() + "'";
		add_match(match.c_str());
	}

        return result;
}


Patch to implement this and add some docs to request_name():

Index: include/dbus-c++/connection.h
===================================================================
RCS file: /usr/cvs/arm-utils/modules/dbus-cpp/include/dbus-c++/connection.h,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 connection.h
--- include/dbus-c++/connection.h	28 Aug 2009 22:18:07 -0000	1.1.1.1
+++ include/dbus-c++/connection.h	14 Sep 2009 19:59:50 -0000
@@ -408,7 +408,19 @@
 	 */
 	PendingCall send_async( Message& msg, int timeout = -1);
 
-	void request_name( const char* name, int flags = 0 );
+	/*!
+	 * \brief Requests ownership of the given name.
+	 *
+	 * The returned result will be one of be one of the normal results of
+         * dbus_bus_request_name(), such as DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER
+	 * or DBUS_DBUS_REQUEST_NAME_REPLY_IN_QUEUE if successful.
+         * See <dbus-1.0/dbus-shared.h>.  Errors are thrown.
+	 * 
+	 * \param name The name to register as the owner of
+         * \param flags Flags such as DBUS_NAME_FLAG_REPLACE_EXISTING
+	 * \throw Error
+         */
+	int request_name( const char* name, int flags = 0 );
     
     unsigned long sender_unix_uid(const char *sender);
 
Index: src/connection.cpp
===================================================================
RCS file: /usr/cvs/arm-utils/modules/dbus-cpp/src/connection.cpp,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 connection.cpp
--- src/connection.cpp	28 Aug 2009 22:18:08 -0000	1.1.1.1
+++ src/connection.cpp	14 Sep 2009 19:59:50 -0000
@@ -378,13 +378,15 @@
 	return PendingCall(new PendingCall::Private(pending));
 }
 
-void Connection::request_name(const char *name, int flags)
+// flags are normal dbus_bus_request_name flags, result is normal return
+int Connection::request_name(const char *name, int flags)
 {
 	InternalError e;
+        int result;
 
 	debug_log("%s: registering bus name %s", unique_name(), name);
 
-	dbus_bus_request_name(_pvt->conn, name, flags, e);	//we deliberately don't check return value
+	result = dbus_bus_request_name(_pvt->conn, name, flags, e);
 
 	if (e) throw Error(e);
 
@@ -396,6 +398,8 @@
 		std::string match = "destination='" + _pvt->names.back() + "'";
 		add_match(match.c_str());
 	}
+
+        return result;
 }
 
 unsigned long Connection::sender_unix_uid(const char *sender)

-- 
Randell Jesup, Worldgate (developers of the Ojo videophone)
rj...@wg...