[PATCH 1/2] log: Add function to get/set debug_string

[PATCH 1/2] log: Add function to get/set debug_string

[Gustavo F. Padovan]

---
 src/log.c |   53 +++++++++++++++++++++++++++++++++++++++++------------
 src/log.h |    2 ++
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/src/log.c b/src/log.c
index ed6e428..f53ace3 100644
--- a/src/log.c
+++ b/src/log.c
@@ -71,6 +71,8 @@ extern struct btd_debug_desc __stop___debug[];
 
 static gchar **enabled = NULL;
 
+static char *debug_string = NULL;
+
 static gboolean is_enabled(struct btd_debug_desc *desc)
 {
         int i;
@@ -90,22 +92,15 @@ static gboolean is_enabled(struct btd_debug_desc *desc)
         return 0;
 }
 
-void __btd_toggle_debug()
-{
-	struct btd_debug_desc *desc;
-
-	for (desc = __start___debug; desc < __stop___debug; desc++)
-		desc->flags |= BTD_DEBUG_FLAG_PRINT;
-}
-
-void __btd_log_init(const char *debug, int detach)
+static void debug_on()
 {
-	int option = LOG_NDELAY | LOG_PID;
 	struct btd_debug_desc *desc;
 	const char *name = NULL, *file = NULL;
 
-	if (debug != NULL)
-		enabled = g_strsplit_set(debug, ":, ", 0);
+	if (debug_string == NULL)
+		debug_string = g_strdup("");
+
+	enabled = g_strsplit_set(debug_string, ":, ", 0);
 
 	for (desc = __start___debug; desc < __stop___debug; desc++) {
 		if (file != NULL || name != NULL) {
@@ -118,7 +113,41 @@ void __btd_log_init(const char *debug, int detach)
 
 		if (is_enabled(desc))
 			desc->flags |= BTD_DEBUG_FLAG_PRINT;
+		else
+			desc->flags &= ~BTD_DEBUG_FLAG_PRINT;
 	}
+}
+
+char *__btd_get_debug()
+{
+	return debug_string;
+}
+
+char *__btd_set_debug(char *str)
+{
+	g_free(debug_string);
+	debug_string = g_strdup(str);
+
+	debug_on();
+
+	return debug_string;
+}
+
+void __btd_toggle_debug()
+{
+	struct btd_debug_desc *desc;
+
+	for (desc = __start___debug; desc < __stop___debug; desc++)
+		desc->flags |= BTD_DEBUG_FLAG_PRINT;
+}
+
+void __btd_log_init(const char *debug, int detach)
+{
+	int option = LOG_NDELAY | LOG_PID;
+
+	debug_string = (char *)debug;
+
+	debug_on();
 
 	if (!detach)
 		option |= LOG_PERROR;
diff --git a/src/log.h b/src/log.h
index 4e0e518..2dfce8d 100644
--- a/src/log.h
+++ b/src/log.h
@@ -28,6 +28,8 @@ void btd_debug(const char *format, ...) __attribute__((format(printf, 1, 2)));
 
 void __btd_log_init(const char *debug, int detach);
 void __btd_log_cleanup(void);
+char *__btd_get_debug();
+char *__btd_set_debug(char *str);
 void __btd_toggle_debug();
 
 struct btd_debug_desc {
-- 
1.7.1

[PATCH 2/2] Add SetProperty method and Debug property

[Gustavo F. Padovan]

---
 doc/manager-api.txt |   16 ++++++++++++
 src/manager.c       |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index d2c1caf..22ee912 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -22,6 +22,15 @@ Methods		dict GetProperties()
 			Possible Errors: org.bluez.Error.DoesNotExist
 					 org.bluez.Error.InvalidArguments
 
+		void SetProperty(string name, variant value)
+
+			Changes the value of the specified property. Only
+			properties that are listed a read-write are changeable.
+			On success this will emit a PropertyChanged signal.
+
+			Possible Errors: org.bluez.Error.DoesNotExist
+					 org.bluez.Error.InvalidArguments
+
 		object DefaultAdapter()
 
 			Returns object path for the default adapter.
@@ -72,3 +81,10 @@ Signals		PropertyChanged(string name, variant value)
 Properties	array{object} Adapters [readonly]
 
 			List of adapter object paths.
+
+		string Debug [readwrite]
+
+			Change the dynamic debug match string. Use it to
+			restrict debug to specific files. The default value is
+			"*" to debug all files. The "" string switch debug
+			off.
diff --git a/src/manager.c b/src/manager.c
index cbbca1e..d146e4c 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -80,6 +80,31 @@ static inline DBusMessage *no_such_adapter(DBusMessage *msg)
 			"No such adapter");
 }
 
+static DBusMessage *set_debug(DBusConnection *conn, DBusMessage *msg,
+					const char *debug_string, void *data)
+{
+	char *old_string;
+
+	if (!g_utf8_validate(debug_string, -1, NULL)) {
+		error("DebugString change failed: supplied string "
+							"isn't valid UTF-8");
+		return invalid_args(msg);
+	}
+
+	old_string = __btd_get_debug();
+
+	if (strcmp((char *)debug_string, old_string) == 0)
+		return dbus_message_new_method_return(msg);
+
+	if (!__btd_set_debug((char *)debug_string))
+		return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
+							strerror(ENOMEM));
+	emit_property_changed(connection, "/", MANAGER_INTERFACE,
+				"Debug", DBUS_TYPE_STRING, &debug_string);
+
+	return dbus_message_new_method_return(msg);
+}
+
 static DBusMessage *default_adapter(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
@@ -183,6 +208,7 @@ static DBusMessage *get_properties(DBusConnection *conn,
 	DBusMessageIter dict;
 	GSList *list;
 	char **array;
+	char *debug_string;
 	int i;
 
 	reply = dbus_message_new_method_return(msg);
@@ -208,13 +234,52 @@ static DBusMessage *get_properties(DBusConnection *conn,
 	dict_append_array(&dict, "Adapters", DBUS_TYPE_OBJECT_PATH, &array, i);
 	g_free(array);
 
+	debug_string = __btd_get_debug();
+	dict_append_entry(&dict, "Debug", DBUS_TYPE_STRING,
+							&debug_string);
+
 	dbus_message_iter_close_container(&iter, &dict);
 
 	return reply;
 }
 
+static DBusMessage *set_property(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	DBusMessageIter iter;
+	DBusMessageIter sub;
+	const char *property;
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &property);
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+		return invalid_args(msg);
+	dbus_message_iter_recurse(&iter, &sub);
+
+	if (g_str_equal("Debug", property)) {
+		const char *string;
+
+		if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
+			return invalid_args(msg);
+		dbus_message_iter_get_basic(&sub, &string);
+
+		return set_debug(conn, msg, string, data);
+	}
+
+	return invalid_args(msg);
+}
+
 static GDBusMethodTable manager_methods[] = {
 	{ "GetProperties",	"",	"a{sv}",get_properties	},
+	{ "SetProperty",	"sv",	"",	set_property,
+						G_DBUS_METHOD_FLAG_ASYNC},
 	{ "DefaultAdapter",	"",	"o",	default_adapter	},
 	{ "FindAdapter",	"s",	"o",	find_adapter	},
 	{ "ListAdapters",	"",	"ao",	list_adapters,
-- 
1.7.1

Re: [PATCH 1/2] log: Add function to get/set debug_string

[Johan Hedberg]

Hi Gustavo,

On Wed, Jun 16, 2010, Gustavo F. Padovan wrote:
> +void __btd_log_init(const char *debug, int detach)
> +{
> +	int option = LOG_NDELAY | LOG_PID;
> +
> +	debug_string = (char *)debug;

This doesn't look right. Either the function declares the parameter as
const for a reason (i.e. it promises not to mess around with it) or then
the const should go away. I think the former is correct in this case,
i.e. you should probably be doing g_strdup on the input parameter.

Johan

Re: [PATCH 2/2] Add SetProperty method and Debug property

[Johan Hedberg]

Hi Gustavo,

On Wed, Jun 16, 2010, Gustavo F. Padovan wrote:
> +static DBusMessage *set_debug(DBusConnection *conn, DBusMessage *msg,
> +					const char *debug_string, void *data)
> +{
> +	char *old_string;
> +
> +	if (!g_utf8_validate(debug_string, -1, NULL)) {
> +		error("DebugString change failed: supplied string "
> +							"isn't valid UTF-8");
> +		return invalid_args(msg);
> +	}
> +
> +	old_string = __btd_get_debug();
> +
> +	if (strcmp((char *)debug_string, old_string) == 0)

At least according to my manpage of strcmp the input is already const so
the case is unnecessary here. You might also want to use g_str_equal in
this case.

> +	if (!__btd_set_debug((char *)debug_string))

Since __btd_set_debug g_strdup's the input the right fix is to declare
its input parameter as const, and then the need to do a cast here goes
away.

Johan

Re: [PATCH 1/2] log: Add function to get/set debug_string

[Marcel Holtmann]

Hi Gustavo,

>  src/log.c |   53 +++++++++++++++++++++++++++++++++++++++++------------
>  src/log.h |    2 ++
>  2 files changed, 43 insertions(+), 12 deletions(-)
> 
> diff --git a/src/log.c b/src/log.c
> index ed6e428..f53ace3 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -71,6 +71,8 @@ extern struct btd_debug_desc __stop___debug[];
>  
>  static gchar **enabled = NULL;
>  
> +static char *debug_string = NULL;
> +

can you please stop messing around with this so much. This looks all so
heavily complicated and convoluted. The log.c is not storing the current
debug command line parameter. It doesn't care about that. So why should
it bother.

Just have a function __btd_enable_debug(const char *debug) and make that
work. You can even use that one from the SIGUSR2 with just "*" as
parameter.

If the D-Bus API wants to present a read-able value of what is the
current string, then that is up to the D-Bus manager object and not the
logging code.

Right now I am actually inclined to just say we don't bother with
changing debug options via D-Bus at all. So I want a simple and clean
patch or we should just not do it at all.

Regards

Marcel