[PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This flag can be used to mark methods as experimental, the marked
methods with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
 gdbus/gdbus.h  | 21 ++++++++++++++++++---
 gdbus/object.c | 17 +++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 0e5c012..00fbb1c 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
 						GDBusPendingReply pending);
 
 enum GDBusMethodFlags {
-	G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
-	G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
-	G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
+	G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
+	G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
+	G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
+	G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
 };
 
 enum GDBusSignalFlags {
@@ -173,6 +174,20 @@ struct GDBusSecurityTable {
 	.function = _function, \
 	.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
 
+#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
+	.name = _name, \
+	.in_args = _in_args, \
+	.out_args = _out_args, \
+	.function = _function, \
+	.flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
+
+#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
+	.name = _name, \
+	.in_args = _in_args, \
+	.out_args = _out_args, \
+	.function = _function, \
+	.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
+
 #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
 	.name = _name, \
 	.in_args = _in_args, \
diff --git a/gdbus/object.c b/gdbus/object.c
index 776d35e..30dbbc2 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 						G_DBUS_METHOD_FLAG_DEPRECATED;
 		gboolean noreply = method->flags &
 						G_DBUS_METHOD_FLAG_NOREPLY;
+		gboolean experimental = method->flags &
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
+
+		if (experimental) {
+			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+			if (g_strcmp0(env, "1") != 0)
+				continue;
+		}
 
 		if (!deprecated && !noreply &&
 				!(method->in_args && method->in_args->name) &&
@@ -1022,10 +1030,19 @@ static DBusHandlerResult generic_message(DBusConnection *connection,
 
 	for (method = iface->methods; method &&
 			method->name && method->function; method++) {
+		gboolean experimental = method->flags &
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
+
 		if (dbus_message_is_method_call(message, iface->name,
 							method->name) == FALSE)
 			continue;
 
+		if (experimental) {
+			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+			if (g_strcmp0(env, "1") != 0)
+				return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+		}
+
 		if (g_dbus_args_have_signature(method->in_args,
 							message) == FALSE)
 			continue;
-- 
1.7.11.7

[PATCH BlueZ 2/8] gdbus: Introduce G_DBUS_SIGNAL_FLAG_EXPERIMENTAL

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This flag can be used to mark signals as experimental, the marked
signals with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
 gdbus/gdbus.h  |  8 +++++++-
 gdbus/object.c | 21 ++++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index 00fbb1c..e6b51cd 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -96,7 +96,8 @@ enum GDBusMethodFlags {
 };
 
 enum GDBusSignalFlags {
-	G_DBUS_SIGNAL_FLAG_DEPRECATED = (1 << 0),
+	G_DBUS_SIGNAL_FLAG_DEPRECATED   = (1 << 0),
+	G_DBUS_SIGNAL_FLAG_EXPERIMENTAL = (1 << 1),
 };
 
 enum GDBusPropertyFlags {
@@ -204,6 +205,11 @@ struct GDBusSecurityTable {
 	.args = _args, \
 	.flags = G_DBUS_SIGNAL_FLAG_DEPRECATED
 
+#define GDBUS_EXPERIMENTAL_SIGNAL(_name, _args) \
+	.name = _name, \
+	.args = _args, \
+	.flags = G_DBUS_SIGNAL_FLAG_EXPERIMENTAL
+
 gboolean g_dbus_register_interface(DBusConnection *connection,
 					const char *path, const char *name,
 					const GDBusMethodTable *methods,
diff --git a/gdbus/object.c b/gdbus/object.c
index 30dbbc2..20f7db9 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -165,6 +165,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 	for (signal = iface->signals; signal && signal->name; signal++) {
 		gboolean deprecated = signal->flags &
 						G_DBUS_SIGNAL_FLAG_DEPRECATED;
+		gboolean experimental = signal->flags &
+					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL;
+
+		if (experimental) {
+			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+			if (g_strcmp0(env, "1") != 0)
+				continue;
+		}
 
 		if (!deprecated && !(signal->args && signal->args->name))
 			g_string_append_printf(gstr,
@@ -1267,10 +1275,17 @@ static gboolean check_signal(DBusConnection *conn, const char *path,
 	}
 
 	for (signal = iface->signals; signal && signal->name; signal++) {
-		if (!strcmp(signal->name, name)) {
-			*args = signal->args;
-			return TRUE;
+		if (strcmp(signal->name, name) != 0)
+			continue;
+
+		if (signal->flags & G_DBUS_SIGNAL_FLAG_EXPERIMENTAL) {
+			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
+			if (g_strcmp0(env, "1") != 0)
+				break;
 		}
+
+		*args = signal->args;
+		return TRUE;
 	}
 
 	error("No signal named %s on interface %s", name, interface);
-- 
1.7.11.7

[PATCH BlueZ 3/8] gdbus: Introduce G_DBUS_PROPERTY_FLAG_EXPERIMENTAL

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This flag can be used to mark properties as experimental, the marked
properties with this flag can be enabled by setting the environment variable
GDBUS_EXPERIMENTAL=1
---
 gdbus/gdbus.h  |  3 ++-
 gdbus/object.c | 60 ++++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index e6b51cd..da3cd53 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -101,7 +101,8 @@ enum GDBusSignalFlags {
 };
 
 enum GDBusPropertyFlags {
-	G_DBUS_PROPERTY_FLAG_DEPRECATED = (1 << 0),
+	G_DBUS_PROPERTY_FLAG_DEPRECATED   = (1 << 0),
+	G_DBUS_PROPERTY_FLAG_EXPERIMENTAL = (1 << 1),
 };
 
 enum GDBusSecurityFlags {
diff --git a/gdbus/object.c b/gdbus/object.c
index 20f7db9..7b58eea 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -118,6 +118,18 @@ static void print_arguments(GString *gstr, const GDBusArgInfo *args,
 #define G_DBUS_ANNOTATE_NOREPLY(prefix_) \
 	G_DBUS_ANNOTATE(prefix_, "Method.NoReply", "true")
 
+static gboolean check_experimental(int flags, int flag)
+{
+	const char *env;
+
+	if (!(flags & flag))
+		return FALSE;
+
+	env = g_getenv("GDBUS_EXPERIMENTAL");
+
+	return g_strcmp0(env, "1") != 0;
+}
+
 static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 {
 	const GDBusMethodTable *method;
@@ -129,14 +141,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 						G_DBUS_METHOD_FLAG_DEPRECATED;
 		gboolean noreply = method->flags &
 						G_DBUS_METHOD_FLAG_NOREPLY;
-		gboolean experimental = method->flags &
-					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
 
-		if (experimental) {
-			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
-			if (g_strcmp0(env, "1") != 0)
-				continue;
-		}
+		if (check_experimental(method->flags,
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+			continue;
 
 		if (!deprecated && !noreply &&
 				!(method->in_args && method->in_args->name) &&
@@ -165,14 +173,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 	for (signal = iface->signals; signal && signal->name; signal++) {
 		gboolean deprecated = signal->flags &
 						G_DBUS_SIGNAL_FLAG_DEPRECATED;
-		gboolean experimental = signal->flags &
-					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL;
 
-		if (experimental) {
-			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
-			if (g_strcmp0(env, "1") != 0)
-				continue;
-		}
+		if (check_experimental(signal->flags,
+					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL))
+			continue;
 
 		if (!deprecated && !(signal->args && signal->args->name))
 			g_string_append_printf(gstr,
@@ -197,6 +201,10 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
 		gboolean deprecated = property->flags &
 					G_DBUS_PROPERTY_FLAG_DEPRECATED;
 
+		if (check_experimental(property->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			continue;
+
 		g_string_append_printf(gstr, "\t\t<property name=\"%s\""
 					" type=\"%s\" access=\"%s%s\"",
 					property->name,	property->type,
@@ -558,6 +566,10 @@ static void append_properties(struct interface_data *data,
 				DBUS_DICT_ENTRY_END_CHAR_AS_STRING, &dict);
 
 	for (p = data->properties; p && p->name; p++) {
+		if (check_experimental(p->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			continue;
+
 		if (p->get == NULL)
 			continue;
 
@@ -759,8 +771,14 @@ static inline const GDBusPropertyTable *find_property(const GDBusPropertyTable *
 	const GDBusPropertyTable *p;
 
 	for (p = properties; p && p->name; p++) {
-		if (strcmp(name, p->name) == 0)
-			return p;
+		if (strcmp(name, p->name) != 0)
+			continue;
+
+		if (check_experimental(p->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			break;
+
+		return p;
 	}
 
 	return NULL;
@@ -1038,18 +1056,14 @@ static DBusHandlerResult generic_message(DBusConnection *connection,
 
 	for (method = iface->methods; method &&
 			method->name && method->function; method++) {
-		gboolean experimental = method->flags &
-					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
 
 		if (dbus_message_is_method_call(message, iface->name,
 							method->name) == FALSE)
 			continue;
 
-		if (experimental) {
-			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
-			if (g_strcmp0(env, "1") != 0)
-				return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-		}
+		if (check_experimental(method->flags,
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+			return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
 		if (g_dbus_args_have_signature(method->in_args,
 							message) == FALSE)
-- 
1.7.11.7

[PATCH BlueZ 4/8] gdbus: Check if the interface being registered is valid

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This prevent registering interfaces that are empty or have all members
marked as experiemental.
---
 gdbus/object.c | 42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 7b58eea..d864bf5 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1177,7 +1177,7 @@ static const GDBusSignalTable manager_signals[] = {
 	{ }
 };
 
-static void add_interface(struct generic_data *data,
+static gboolean add_interface(struct generic_data *data,
 				const char *name,
 				const GDBusMethodTable *methods,
 				const GDBusSignalTable *signals,
@@ -1186,7 +1186,32 @@ static void add_interface(struct generic_data *data,
 				GDBusDestroyFunction destroy)
 {
 	struct interface_data *iface;
+	const GDBusMethodTable *method;
+	const GDBusSignalTable *signal;
+	const GDBusPropertyTable *property;
+
+	for (method = methods; method && method->name; method++) {
+		if (!check_experimental(method->flags,
+					G_DBUS_METHOD_FLAG_EXPERIMENTAL))
+			goto done;
+	}
+
+	for (signal = signals; signal && signal->name; signal++) {
+		if (!check_experimental(signal->flags,
+					G_DBUS_SIGNAL_FLAG_EXPERIMENTAL))
+			goto done;
+	}
+
+	for (property = properties; property && property->name; property++) {
+		if (!check_experimental(property->flags,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL))
+			goto done;
+	}
 
+	/* Nothing to register */
+	return FALSE;
+
+done:
 	iface = g_new0(struct interface_data, 1);
 	iface->name = g_strdup(name);
 	iface->methods = methods;
@@ -1197,13 +1222,15 @@ static void add_interface(struct generic_data *data,
 
 	data->interfaces = g_slist_append(data->interfaces, iface);
 	if (data->parent == NULL)
-		return;
+		return TRUE;
 
 	data->added = g_slist_append(data->added, iface);
 	if (data->process_id > 0)
-		return;
+		return TRUE;
 
 	data->process_id = g_idle_add(process_changes, data);
+
+	return TRUE;
 }
 
 static struct generic_data *object_path_ref(DBusConnection *connection,
@@ -1364,15 +1391,18 @@ gboolean g_dbus_register_interface(DBusConnection *connection,
 		return FALSE;
 	}
 
+	if (!add_interface(data, name, methods, signals, properties, user_data,
+								destroy)) {
+		object_path_unref(connection, path);
+		return FALSE;
+	}
+
 	if (properties != NULL && !find_interface(data->interfaces,
 						DBUS_INTERFACE_PROPERTIES))
 		add_interface(data, DBUS_INTERFACE_PROPERTIES,
 				properties_methods, properties_signals, NULL,
 				data, NULL);
 
-	add_interface(data, name, methods, signals, properties, user_data,
-								destroy);
-
 	g_free(data->introspect);
 	data->introspect = NULL;
 
-- 
1.7.11.7

[PATCH BlueZ 5/8] gdbus: Call check_signals when sending signals with g_dbus_send_message

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

If message passed to g_dbus_send_message is a signal verify if it is a
valid and there really exists an interface with respective signal name.
---
 gdbus/object.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gdbus/object.c b/gdbus/object.c
index d864bf5..565dd6f 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1514,6 +1514,15 @@ gboolean g_dbus_send_message(DBusConnection *connection, DBusMessage *message)
 
 	if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_METHOD_CALL)
 		dbus_message_set_no_reply(message, TRUE);
+	else if (dbus_message_get_type(message) == DBUS_MESSAGE_TYPE_SIGNAL) {
+		const char *path = dbus_message_get_path(message);
+		const char *interface = dbus_message_get_interface(message);
+		const char *name = dbus_message_get_member(message);
+		const GDBusArgInfo *args;
+
+		if (!check_signal(connection, path, interface, name, &args))
+			return FALSE;
+	}
 
 	result = dbus_connection_send(connection, message, NULL);
 
-- 
1.7.11.7

[PATCH BlueZ 6/8] media: Enable RegisterPlayer and UnregisterPlayer methods as experimental

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

---
 profiles/audio/media.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index f728460..e4206e3 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -868,7 +868,6 @@ static DBusMessage *unregister_endpoint(DBusConnection *conn, DBusMessage *msg,
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
 
-#if 0
 static struct media_player *media_adapter_find_player(
 						struct media_adapter *adapter,
 						const char *sender,
@@ -1533,7 +1532,6 @@ static DBusMessage *unregister_player(DBusConnection *conn, DBusMessage *msg,
 
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
-#endif
 
 static const GDBusMethodTable media_methods[] = {
 	{ GDBUS_METHOD("RegisterEndpoint",
@@ -1541,14 +1539,12 @@ static const GDBusMethodTable media_methods[] = {
 		NULL, register_endpoint) },
 	{ GDBUS_METHOD("UnregisterEndpoint",
 		GDBUS_ARGS({ "endpoint", "o" }), NULL, unregister_endpoint) },
-#if 0
-	{ GDBUS_METHOD("RegisterPlayer",
+	{ GDBUS_EXPERIMENTAL_METHOD("RegisterPlayer",
 		GDBUS_ARGS({ "player", "o" }, { "properties", "a{sv}" },
 						{ "metadata", "a{sv}" }),
 		NULL, register_player) },
-	{ GDBUS_METHOD("UnregisterPlayer",
+	{ GDBUS_EXPERIMENTAL_METHOD("UnregisterPlayer",
 		GDBUS_ARGS({ "player", "o" }), NULL, unregister_player) },
-#endif
 	{ },
 };
 
@@ -1559,10 +1555,8 @@ static void path_free(void *data)
 	while (adapter->endpoints)
 		release_endpoint(adapter->endpoints->data);
 
-#if 0
 	while (adapter->players)
 		media_player_destroy(adapter->players->data);
-#endif
 
 	adapters = g_slist_remove(adapters, adapter);
 
-- 
1.7.11.7

[PATCH BlueZ 7/8] player: Enable MediaPlayer1 interface as experimental

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

This enable MediaPlayer1 when GDBUS_EXPERIMENTAL=1
---
 profiles/audio/player.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/profiles/audio/player.c b/profiles/audio/player.c
index 005d0d1..693a590 100644
--- a/profiles/audio/player.c
+++ b/profiles/audio/player.c
@@ -239,25 +239,31 @@ static void set_setting(const GDBusPropertyTable *property,
 }
 
 static const GDBusMethodTable media_player_methods[] = {
-	{ GDBUS_METHOD("GetTrack",
+	{ GDBUS_EXPERIMENTAL_METHOD("GetTrack",
 			NULL, GDBUS_ARGS({ "metadata", "a{sv}" }),
 			media_player_get_track) },
 	{ }
 };
 
 static const GDBusSignalTable media_player_signals[] = {
-	{ GDBUS_SIGNAL("TrackChanged",
+	{ GDBUS_EXPERIMENTAL_SIGNAL("TrackChanged",
 			GDBUS_ARGS({ "metadata", "a{sv}" })) },
 	{ }
 };
 
 static const GDBusPropertyTable media_player_properties[] = {
-	{ "Position", "u", get_position },
-	{ "Status", "s", get_status, NULL, status_exists },
-	{ "Equalizer", "s", get_setting, set_setting, setting_exists },
-	{ "Repeat", "s", get_setting, set_setting, setting_exists },
-	{ "Shuffle", "s", get_setting, set_setting, setting_exists },
-	{ "Scan", "s", get_setting, set_setting, setting_exists },
+	{ "Position", "u", get_position, NULL, NULL,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Status", "s", get_status, NULL, status_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Equalizer", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Repeat", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Shuffle", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
+	{ "Scan", "s", get_setting, set_setting, setting_exists,
+					G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ }
 };
 
@@ -298,7 +304,6 @@ struct media_player *media_player_controller_create(const char *path)
 							g_free, g_free);
 	mp->progress = g_timer_new();
 
-#if 0
 	if (!g_dbus_register_interface(btd_get_dbus_connection(),
 					mp->path, MEDIA_PLAYER_INTERFACE,
 					media_player_methods,
@@ -308,7 +313,7 @@ struct media_player *media_player_controller_create(const char *path)
 		media_player_destroy(mp);
 		return NULL;
 	}
-#endif
+
 	DBG("%s", mp->path);
 
 	return mp;
@@ -410,7 +415,6 @@ void media_player_set_status(struct media_player *mp, const char *status)
 
 static gboolean process_metadata_changed(void *user_data)
 {
-#if 0
 	struct media_player *mp = user_data;
 	DBusMessage *signal;
 	DBusMessageIter iter, dict;
@@ -439,7 +443,7 @@ static gboolean process_metadata_changed(void *user_data)
 	dbus_message_iter_close_container(&iter, &dict);
 
 	g_dbus_send_message(btd_get_dbus_connection(), signal);
-#endif
+
 	return FALSE;
 }
 
-- 
1.7.11.7

[PATCH BlueZ 8/8] AVRCP: Fix not checking for media_player_controller_create

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

Now that the MediaPlayer1 interface is experimental the interface
registration may fail which return NULL, in that case there is no
point on register to any notifications.
---
 profiles/audio/avrcp.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/profiles/audio/avrcp.c b/profiles/audio/avrcp.c
index 4e3d31d..ce070cd 100644
--- a/profiles/audio/avrcp.c
+++ b/profiles/audio/avrcp.c
@@ -1987,6 +1987,11 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
 
 	count = pdu->params[1];
 
+	path = device_get_path(session->dev->btd_dev);
+	mp = media_player_controller_create(path);
+	if (mp == NULL)
+		return FALSE;
+
 	for (; count > 0; count--) {
 		uint8_t event = pdu->params[1 + count];
 
@@ -2001,8 +2006,6 @@ static gboolean avrcp_get_capabilities_resp(struct avctp *conn,
 		}
 	}
 
-	path = device_get_path(session->dev->btd_dev);
-	mp = media_player_controller_create(path);
 	media_player_set_callbacks(mp, &ct_cbs, player);
 	player->user_data = mp;
 	player->destroy = (GDestroyNotify) media_player_destroy;
-- 
1.7.11.7

Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[Marcel Holtmann]

Hi Luiz,

> This flag can be used to mark methods as experimental, the marked
> methods with this flag can be enabled by setting the environment variable
> GDBUS_EXPERIMENTAL=1
> ---
>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
>  gdbus/object.c | 17 +++++++++++++++++
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> index 0e5c012..00fbb1c 100644
> --- a/gdbus/gdbus.h
> +++ b/gdbus/gdbus.h
> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>  						GDBusPendingReply pending);
>  
>  enum GDBusMethodFlags {
> -	G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> -	G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
> -	G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
> +	G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
> +	G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
> +	G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
> +	G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>  };
>  
>  enum GDBusSignalFlags {
> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>  	.function = _function, \
>  	.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>  
> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
> +	.name = _name, \
> +	.in_args = _in_args, \
> +	.out_args = _out_args, \
> +	.function = _function, \
> +	.flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
> +
> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
> +	.name = _name, \
> +	.in_args = _in_args, \
> +	.out_args = _out_args, \
> +	.function = _function, \
> +	.flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
> +
>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>  	.name = _name, \
>  	.in_args = _in_args, \
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 776d35e..30dbbc2 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>  						G_DBUS_METHOD_FLAG_DEPRECATED;
>  		gboolean noreply = method->flags &
>  						G_DBUS_METHOD_FLAG_NOREPLY;
> +		gboolean experimental = method->flags &
> +					G_DBUS_METHOD_FLAG_EXPERIMENTAL;
> +
> +		if (experimental) {
> +			const char *env = g_getenv("GDBUS_EXPERIMENTAL");
> +			if (g_strcmp0(env, "1") != 0)
> +				continue;
> +		}

actually since this is a library, doing it this way is a bad idea.

Lets do something like g_dbus_enable_experimental(DBusConnection)

Regards

Marcel

Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[Luiz Augusto von Dentz]

Hi Marcel,

On Sun, Dec 23, 2012 at 10:58 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> This flag can be used to mark methods as experimental, the marked
>> methods with this flag can be enabled by setting the environment variable
>> GDBUS_EXPERIMENTAL=1
>> ---
>>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
>>  gdbus/object.c | 17 +++++++++++++++++
>>  2 files changed, 35 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> index 0e5c012..00fbb1c 100644
>> --- a/gdbus/gdbus.h
>> +++ b/gdbus/gdbus.h
>> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>>                                               GDBusPendingReply pending);
>>
>>  enum GDBusMethodFlags {
>> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
>> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
>> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
>> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
>> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
>> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
>> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>>  };
>>
>>  enum GDBusSignalFlags {
>> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>>       .function = _function, \
>>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>>
>> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
>> +     .name = _name, \
>> +     .in_args = _in_args, \
>> +     .out_args = _out_args, \
>> +     .function = _function, \
>> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> +
>> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
>> +     .name = _name, \
>> +     .in_args = _in_args, \
>> +     .out_args = _out_args, \
>> +     .function = _function, \
>> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> +
>>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>>       .name = _name, \
>>       .in_args = _in_args, \
>> diff --git a/gdbus/object.c b/gdbus/object.c
>> index 776d35e..30dbbc2 100644
>> --- a/gdbus/object.c
>> +++ b/gdbus/object.c
>> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
>>               gboolean noreply = method->flags &
>>                                               G_DBUS_METHOD_FLAG_NOREPLY;
>> +             gboolean experimental = method->flags &
>> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
>> +
>> +             if (experimental) {
>> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
>> +                     if (g_strcmp0(env, "1") != 0)
>> +                             continue;
>> +             }
>
> actually since this is a library, doing it this way is a bad idea.

I thought it was a common practice to use environment variables with
libraries to change certain defaults, glib does that with things like
G_SLICE=always-malloc, and it is quite convenient since you can change
easily without recompiling.

> Lets do something like g_dbus_enable_experimental(DBusConnection)

But this is not really per connection, anyway doing so you have to
handle this directly on the application code which IMO is not as
convenient.

--
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[Marcel Holtmann]

Hi Luiz,

> >> This flag can be used to mark methods as experimental, the marked
> >> methods with this flag can be enabled by setting the environment variable
> >> GDBUS_EXPERIMENTAL=1
> >> ---
> >>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
> >>  gdbus/object.c | 17 +++++++++++++++++
> >>  2 files changed, 35 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> >> index 0e5c012..00fbb1c 100644
> >> --- a/gdbus/gdbus.h
> >> +++ b/gdbus/gdbus.h
> >> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
> >>                                               GDBusPendingReply pending);
> >>
> >>  enum GDBusMethodFlags {
> >> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> >> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
> >> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
> >> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
> >> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
> >> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
> >> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
> >>  };
> >>
> >>  enum GDBusSignalFlags {
> >> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
> >>       .function = _function, \
> >>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
> >>
> >> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
> >> +     .name = _name, \
> >> +     .in_args = _in_args, \
> >> +     .out_args = _out_args, \
> >> +     .function = _function, \
> >> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
> >> +
> >> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
> >> +     .name = _name, \
> >> +     .in_args = _in_args, \
> >> +     .out_args = _out_args, \
> >> +     .function = _function, \
> >> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
> >> +
> >>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
> >>       .name = _name, \
> >>       .in_args = _in_args, \
> >> diff --git a/gdbus/object.c b/gdbus/object.c
> >> index 776d35e..30dbbc2 100644
> >> --- a/gdbus/object.c
> >> +++ b/gdbus/object.c
> >> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
> >>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
> >>               gboolean noreply = method->flags &
> >>                                               G_DBUS_METHOD_FLAG_NOREPLY;
> >> +             gboolean experimental = method->flags &
> >> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
> >> +
> >> +             if (experimental) {
> >> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
> >> +                     if (g_strcmp0(env, "1") != 0)
> >> +                             continue;
> >> +             }
> >
> > actually since this is a library, doing it this way is a bad idea.
> 
> I thought it was a common practice to use environment variables with
> libraries to change certain defaults, glib does that with things like
> G_SLICE=always-malloc, and it is quite convenient since you can change
> easily without recompiling.

GLib does this, but we never did this. GAtChat, GDHCP, GWeb etc.
provided a function to enable it. The hooking up to environment variable
is then the responsibility of the main program.

> > Lets do something like g_dbus_enable_experimental(DBusConnection)
> 
> But this is not really per connection, anyway doing so you have to
> handle this directly on the application code which IMO is not as
> convenient.

Making this per connection would be pretty convenient if you are
connected to more than one bus.

Regards

Marcel

Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[Luiz Augusto von Dentz]

HI Marcel,

On Mon, Dec 24, 2012 at 1:35 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Luiz,
>
>> >> This flag can be used to mark methods as experimental, the marked
>> >> methods with this flag can be enabled by setting the environment variable
>> >> GDBUS_EXPERIMENTAL=1
>> >> ---
>> >>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
>> >>  gdbus/object.c | 17 +++++++++++++++++
>> >>  2 files changed, 35 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>> >> index 0e5c012..00fbb1c 100644
>> >> --- a/gdbus/gdbus.h
>> >> +++ b/gdbus/gdbus.h
>> >> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>> >>                                               GDBusPendingReply pending);
>> >>
>> >>  enum GDBusMethodFlags {
>> >> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
>> >> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
>> >> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
>> >> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
>> >> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
>> >> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
>> >> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>> >>  };
>> >>
>> >>  enum GDBusSignalFlags {
>> >> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>> >>       .function = _function, \
>> >>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>> >>
>> >> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
>> >> +     .name = _name, \
>> >> +     .in_args = _in_args, \
>> >> +     .out_args = _out_args, \
>> >> +     .function = _function, \
>> >> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> >> +
>> >> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
>> >> +     .name = _name, \
>> >> +     .in_args = _in_args, \
>> >> +     .out_args = _out_args, \
>> >> +     .function = _function, \
>> >> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
>> >> +
>> >>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>> >>       .name = _name, \
>> >>       .in_args = _in_args, \
>> >> diff --git a/gdbus/object.c b/gdbus/object.c
>> >> index 776d35e..30dbbc2 100644
>> >> --- a/gdbus/object.c
>> >> +++ b/gdbus/object.c
>> >> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>> >>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
>> >>               gboolean noreply = method->flags &
>> >>                                               G_DBUS_METHOD_FLAG_NOREPLY;
>> >> +             gboolean experimental = method->flags &
>> >> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
>> >> +
>> >> +             if (experimental) {
>> >> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
>> >> +                     if (g_strcmp0(env, "1") != 0)
>> >> +                             continue;
>> >> +             }
>> >
>> > actually since this is a library, doing it this way is a bad idea.
>>
>> I thought it was a common practice to use environment variables with
>> libraries to change certain defaults, glib does that with things like
>> G_SLICE=always-malloc, and it is quite convenient since you can change
>> easily without recompiling.
>
> GLib does this, but we never did this. GAtChat, GDHCP, GWeb etc.
> provided a function to enable it. The hooking up to environment variable
> is then the responsibility of the main program.

GObex does have it on environment variables and I can even enable them
while running make check so if a test fail I can debug like the daemon
itself.

>> > Lets do something like g_dbus_enable_experimental(DBusConnection)
>>
>> But this is not really per connection, anyway doing so you have to
>> handle this directly on the application code which IMO is not as
>> convenient.
>
> Making this per connection would be pretty convenient if you are
> connected to more than one bus.

Except that we don't actually change during runtime to be able to use
the connection and it would probably confuse applications that already
read the introspection data if we do so.



--
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[Luiz Augusto von Dentz]

Hi Marcel,

On Mon, Dec 24, 2012 at 2:12 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> HI Marcel,
>
> On Mon, Dec 24, 2012 at 1:35 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> Hi Luiz,
>>
>>> >> This flag can be used to mark methods as experimental, the marked
>>> >> methods with this flag can be enabled by setting the environment variable
>>> >> GDBUS_EXPERIMENTAL=1
>>> >> ---
>>> >>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
>>> >>  gdbus/object.c | 17 +++++++++++++++++
>>> >>  2 files changed, 35 insertions(+), 3 deletions(-)
>>> >>
>>> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
>>> >> index 0e5c012..00fbb1c 100644
>>> >> --- a/gdbus/gdbus.h
>>> >> +++ b/gdbus/gdbus.h
>>> >> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
>>> >>                                               GDBusPendingReply pending);
>>> >>
>>> >>  enum GDBusMethodFlags {
>>> >> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
>>> >> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
>>> >> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
>>> >> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
>>> >> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
>>> >> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
>>> >> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
>>> >>  };
>>> >>
>>> >>  enum GDBusSignalFlags {
>>> >> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
>>> >>       .function = _function, \
>>> >>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
>>> >>
>>> >> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
>>> >> +     .name = _name, \
>>> >> +     .in_args = _in_args, \
>>> >> +     .out_args = _out_args, \
>>> >> +     .function = _function, \
>>> >> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
>>> >> +
>>> >> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
>>> >> +     .name = _name, \
>>> >> +     .in_args = _in_args, \
>>> >> +     .out_args = _out_args, \
>>> >> +     .function = _function, \
>>> >> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
>>> >> +
>>> >>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
>>> >>       .name = _name, \
>>> >>       .in_args = _in_args, \
>>> >> diff --git a/gdbus/object.c b/gdbus/object.c
>>> >> index 776d35e..30dbbc2 100644
>>> >> --- a/gdbus/object.c
>>> >> +++ b/gdbus/object.c
>>> >> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
>>> >>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
>>> >>               gboolean noreply = method->flags &
>>> >>                                               G_DBUS_METHOD_FLAG_NOREPLY;
>>> >> +             gboolean experimental = method->flags &
>>> >> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
>>> >> +
>>> >> +             if (experimental) {
>>> >> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
>>> >> +                     if (g_strcmp0(env, "1") != 0)
>>> >> +                             continue;
>>> >> +             }
>>> >
>>> > actually since this is a library, doing it this way is a bad idea.
>>>
>>> I thought it was a common practice to use environment variables with
>>> libraries to change certain defaults, glib does that with things like
>>> G_SLICE=always-malloc, and it is quite convenient since you can change
>>> easily without recompiling.
>>
>> GLib does this, but we never did this. GAtChat, GDHCP, GWeb etc.
>> provided a function to enable it. The hooking up to environment variable
>> is then the responsibility of the main program.
>
> GObex does have it on environment variables and I can even enable them
> while running make check so if a test fail I can debug like the daemon
> itself.
>
>>> > Lets do something like g_dbus_enable_experimental(DBusConnection)
>>>
>>> But this is not really per connection, anyway doing so you have to
>>> handle this directly on the application code which IMO is not as
>>> convenient.
>>
>> Making this per connection would be pretty convenient if you are
>> connected to more than one bus.
>
> Except that we don't actually change during runtime to be able to use
> the connection and it would probably confuse applications that already
> read the introspection data if we do so.

Now that the release is out I guess we should try to get this changes
in, I would like to make similar changes to disable deprecated in a
similar way using, so we probably have to settle on a way this should
be done. Since we diverge in the way to use environment variable I was
thinking in an alternative, what about using binary parameter e.g.
-E/--experimental and -D/--deprecated?

--
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/8] gdbus: Introduce G_DBUS_METHOD_FLAG_EXPERIMENTAL

[Marcel Holtmann]

Hi Luiz,

> >>> >> This flag can be used to mark methods as experimental, the marked
> >>> >> methods with this flag can be enabled by setting the environment variable
> >>> >> GDBUS_EXPERIMENTAL=1
> >>> >> ---
> >>> >>  gdbus/gdbus.h  | 21 ++++++++++++++++++---
> >>> >>  gdbus/object.c | 17 +++++++++++++++++
> >>> >>  2 files changed, 35 insertions(+), 3 deletions(-)
> >>> >>
> >>> >> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> >>> >> index 0e5c012..00fbb1c 100644
> >>> >> --- a/gdbus/gdbus.h
> >>> >> +++ b/gdbus/gdbus.h
> >>> >> @@ -89,9 +89,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection *connection,
> >>> >>                                               GDBusPendingReply pending);
> >>> >>
> >>> >>  enum GDBusMethodFlags {
> >>> >> -     G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> >>> >> -     G_DBUS_METHOD_FLAG_NOREPLY    = (1 << 1),
> >>> >> -     G_DBUS_METHOD_FLAG_ASYNC      = (1 << 2),
> >>> >> +     G_DBUS_METHOD_FLAG_DEPRECATED   = (1 << 0),
> >>> >> +     G_DBUS_METHOD_FLAG_NOREPLY      = (1 << 1),
> >>> >> +     G_DBUS_METHOD_FLAG_ASYNC        = (1 << 2),
> >>> >> +     G_DBUS_METHOD_FLAG_EXPERIMENTAL = (1 << 3),
> >>> >>  };
> >>> >>
> >>> >>  enum GDBusSignalFlags {
> >>> >> @@ -173,6 +174,20 @@ struct GDBusSecurityTable {
> >>> >>       .function = _function, \
> >>> >>       .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_DEPRECATED
> >>> >>
> >>> >> +#define GDBUS_EXPERIMENTAL_METHOD(_name, _in_args, _out_args, _function) \
> >>> >> +     .name = _name, \
> >>> >> +     .in_args = _in_args, \
> >>> >> +     .out_args = _out_args, \
> >>> >> +     .function = _function, \
> >>> >> +     .flags = G_DBUS_METHOD_FLAG_EXPERIMENTAL
> >>> >> +
> >>> >> +#define GDBUS_EXPERIMENTAL_ASYNC_METHOD(_name, _in_args, _out_args, _function) \
> >>> >> +     .name = _name, \
> >>> >> +     .in_args = _in_args, \
> >>> >> +     .out_args = _out_args, \
> >>> >> +     .function = _function, \
> >>> >> +     .flags = G_DBUS_METHOD_FLAG_ASYNC | G_DBUS_METHOD_FLAG_EXPERIMENTAL
> >>> >> +
> >>> >>  #define GDBUS_NOREPLY_METHOD(_name, _in_args, _out_args, _function) \
> >>> >>       .name = _name, \
> >>> >>       .in_args = _in_args, \
> >>> >> diff --git a/gdbus/object.c b/gdbus/object.c
> >>> >> index 776d35e..30dbbc2 100644
> >>> >> --- a/gdbus/object.c
> >>> >> +++ b/gdbus/object.c
> >>> >> @@ -129,6 +129,14 @@ static void generate_interface_xml(GString *gstr, struct interface_data *iface)
> >>> >>                                               G_DBUS_METHOD_FLAG_DEPRECATED;
> >>> >>               gboolean noreply = method->flags &
> >>> >>                                               G_DBUS_METHOD_FLAG_NOREPLY;
> >>> >> +             gboolean experimental = method->flags &
> >>> >> +                                     G_DBUS_METHOD_FLAG_EXPERIMENTAL;
> >>> >> +
> >>> >> +             if (experimental) {
> >>> >> +                     const char *env = g_getenv("GDBUS_EXPERIMENTAL");
> >>> >> +                     if (g_strcmp0(env, "1") != 0)
> >>> >> +                             continue;
> >>> >> +             }
> >>> >
> >>> > actually since this is a library, doing it this way is a bad idea.
> >>>
> >>> I thought it was a common practice to use environment variables with
> >>> libraries to change certain defaults, glib does that with things like
> >>> G_SLICE=always-malloc, and it is quite convenient since you can change
> >>> easily without recompiling.
> >>
> >> GLib does this, but we never did this. GAtChat, GDHCP, GWeb etc.
> >> provided a function to enable it. The hooking up to environment variable
> >> is then the responsibility of the main program.
> >
> > GObex does have it on environment variables and I can even enable them
> > while running make check so if a test fail I can debug like the daemon
> > itself.
> >
> >>> > Lets do something like g_dbus_enable_experimental(DBusConnection)
> >>>
> >>> But this is not really per connection, anyway doing so you have to
> >>> handle this directly on the application code which IMO is not as
> >>> convenient.
> >>
> >> Making this per connection would be pretty convenient if you are
> >> connected to more than one bus.
> >
> > Except that we don't actually change during runtime to be able to use
> > the connection and it would probably confuse applications that already
> > read the introspection data if we do so.
> 
> Now that the release is out I guess we should try to get this changes
> in, I would like to make similar changes to disable deprecated in a
> similar way using, so we probably have to settle on a way this should
> be done. Since we diverge in the way to use environment variable I was
> thinking in an alternative, what about using binary parameter e.g.
> -E/--experimental and -D/--deprecated?

I dislike the library to take environment variables to change behavior.
While gobex might do this, none of the other helpers do it that way. The
main program checks for the environment variable and then sets it. So we
should do it the same way.

That said, yes, having a command line switch for the daemon seems
sensible to enable experimental interfaces or disable deprecated ones.

Regards

Marcel