[PATCH RESEND] add zoned debug support
Unfortunately, I can't think of a way to support dynamic flag modification
(other than perhaps adding a dbus method to do it).
>From 6b9569c22193a1d0c8d8d6f38692d5f8f43429c9 Mon Sep 17 00:00:00 2001
From: Andres Salomon
Date: Sat, 8 Aug 2009 20:06:12 -0400
Subject: [PATCH] ofono: add zoned debug support
This adds debug flags so that when users are debugging, they can pass
arguments to --debug to specify what they want shown. --debug without
any args defaults to prior behavior.
---
include/log.h | 11 +--
src/log.c | 11 +++
src/main.c| 25 +++--
src/ofono.h |2 +-
4 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/include/log.h b/include/log.h
index 47e5ec8..8a82f13 100644
--- a/include/log.h
+++ b/include/log.h
@@ -26,6 +26,10 @@
extern "C" {
#endif
+typedef enum {
+ OFONO_DEBUG_CORE = 1 << 0,
+} ofono_debug_flags;
+
/**
* SECTION:log
* @title: Logging premitives
@@ -36,8 +40,11 @@ extern void ofono_info(const char *format, ...)
__attribute__((format(printf, 1, 2)));
extern void ofono_error(const char *format, ...)
__attribute__((format(printf, 1, 2)));
-extern void ofono_debug(const char *format, ...)
- __attribute__((format(printf, 1, 2)));
+extern void __ofono_debug(ofono_debug_flags flag, const char *format, ...)
+ __attribute__((format(printf, 2, 3)));
+#define ofono_debug(format, ...) \
+ __ofono_debug(OFONO_DEBUG_CORE, (format), ##__VA_ARGS__)
+
/**
* DBG:
diff --git a/src/log.c b/src/log.c
index 273e3ba..167fe21 100644
--- a/src/log.c
+++ b/src/log.c
@@ -29,6 +29,7 @@
#include "ofono.h"
static volatile gboolean debug_enabled = FALSE;
+static guint debug_flags;
/**
* ofono_info:
@@ -67,7 +68,8 @@ void ofono_error(const char *format, ...)
}
/**
- * ofono_debug:
+ * __ofono_debug:
+ * @flag: zone flag (ie, OFONO_DEBUG_CORE)
* @format: format string
* @varargs: list of arguments
*
@@ -76,11 +78,11 @@ void ofono_error(const char *format, ...)
* The actual output of the debug message is controlled via a command line
* switch. If not enabled, these messages will be ignored.
*/
-void ofono_debug(const char *format, ...)
+void __ofono_debug(ofono_debug_flags flag, const char *format, ...)
{
va_list ap;
- if (debug_enabled == FALSE)
+ if (!debug_enabled || !(debug_flags & flag))
return;
va_start(ap, format);
@@ -98,7 +100,7 @@ void __ofono_toggle_debug(void)
debug_enabled = TRUE;
}
-int __ofono_log_init(gboolean detach, gboolean debug)
+int __ofono_log_init(gboolean detach, gboolean debug, guint dflags)
{
int option = LOG_NDELAY | LOG_PID;
@@ -110,6 +112,7 @@ int __ofono_log_init(gboolean detach, gboolean debug)
syslog(LOG_INFO, "oFono version %s", VERSION);
debug_enabled = debug;
+ debug_flags = dflags;
return 0;
}
diff --git a/src/main.c b/src/main.c
index 7542e13..7227bde 100644
--- a/src/main.c
+++ b/src/main.c
@@ -54,12 +54,33 @@ static void system_bus_disconnected(DBusConnection *conn,
void *user_data)
static gboolean option_detach = TRUE;
static gboolean option_debug = FALSE;
+static guint debug_flags = 0;
+
+static GDebugKey keys[] = {
+ { "core", OFONO_DEBUG_CORE },
+};
+
+static gboolean parse_debug_flags(const gchar *option_name, const gchar *value,
+ gpointer data, GError **err)
+{
+ option_debug = TRUE;
+
+ /* NULL means no string was supplied to --debug. We default to "core"
+* in that scenario; perhaps we should be defaulting to "all" instead?
*/
+ if (!value)
+ value = "core";
+
+ debug_flags = g_parse_debug_string(value, keys,
+ sizeof(keys) / sizeof(keys[0]));
+ return TRUE;
+}
static GOptionEntry options[] = {
{ "nodetach", 'n', G_OPTION_FLAG_REVERSE,
G_OPTION_ARG_NONE, &option_detach,
"Don't run as daemon in background" },
- { "debug", 'd', 0, G_OPTION_ARG_NONE, &option_debug,
+ { "debug", 'd', G_OPTION_FLAG_OPTIONAL_ARG,
+ G_OPTION_ARG_CALLBACK, &parse_debug_flags,
"Enable debug information output" },
{ NULL },
};
@@ -109,7 +130,7 @@ int main(int argc, char **argv)
}
#endif
- __ofono_log_init(option_detach, option_debug);
+ __ofono_log_init(option_detach, option_debug, debug_flags);
dbus_error_init(&error);
diff --git a/src/ofono.h b/src/ofono.h
index 63f33ad..c791c2d 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -30,7 +30,7 @@ const char **__ofono_modem_get_list();
#include
-int __ofono_log_init(gboolean detach, gboolean debug);
+int __ofono_log_init(gboolean detach, gboolean debug, guint dflags);
void __ofono_log_
Re: [PATCH RESEND] add zoned debug support
Hi Andres,
> This adds debug flags so that when users are debugging, they can pass
> arguments to --debug to specify what they want shown. --debug without
> any args defaults to prior behavior.
> ---
> include/log.h | 11 +--
> src/log.c | 11 +++
> src/main.c| 25 +++--
> src/ofono.h |2 +-
> 4 files changed, 40 insertions(+), 9 deletions(-)
>
> diff --git a/include/log.h b/include/log.h
> index 47e5ec8..8a82f13 100644
> --- a/include/log.h
> +++ b/include/log.h
> @@ -26,6 +26,10 @@
> extern "C" {
> #endif
>
> +typedef enum {
> + OFONO_DEBUG_CORE = 1 << 0,
> +} ofono_debug_flags;
> +
> /**
> * SECTION:log
> * @title: Logging premitives
> @@ -36,8 +40,11 @@ extern void ofono_info(const char *format, ...)
> __attribute__((format(printf, 1, 2)));
> extern void ofono_error(const char *format, ...)
> __attribute__((format(printf, 1, 2)));
> -extern void ofono_debug(const char *format, ...)
> - __attribute__((format(printf, 1, 2)));
> +extern void __ofono_debug(ofono_debug_flags flag, const char *format, ...)
> + __attribute__((format(printf, 2, 3)));
> +#define ofono_debug(format, ...) \
> + __ofono_debug(OFONO_DEBUG_CORE, (format), ##__VA_ARGS__)
> +
I don't like this. __ofono functions should not be part of public header
files.
Instead of trying to workaround previous users, just fix the users and
provide the default zone for DBG().
> /**
> * DBG:
> diff --git a/src/log.c b/src/log.c
> index 273e3ba..167fe21 100644
> --- a/src/log.c
> +++ b/src/log.c
> @@ -29,6 +29,7 @@
> #include "ofono.h"
>
> static volatile gboolean debug_enabled = FALSE;
> +static guint debug_flags;
>
> /**
> * ofono_info:
> @@ -67,7 +68,8 @@ void ofono_error(const char *format, ...)
> }
>
> /**
> - * ofono_debug:
> + * __ofono_debug:
> + * @flag: zone flag (ie, OFONO_DEBUG_CORE)
> * @format: format string
> * @varargs: list of arguments
> *
> @@ -76,11 +78,11 @@ void ofono_error(const char *format, ...)
> * The actual output of the debug message is controlled via a command line
> * switch. If not enabled, these messages will be ignored.
> */
> -void ofono_debug(const char *format, ...)
> +void __ofono_debug(ofono_debug_flags flag, const char *format, ...)
> {
Why not just using unsigned long here. I don't like the idea of
overloading enum with bitmask here.
Actually the more I think about it, do we wanna have multiple zones per
debug message. That sounds way to complicated anyway. So why not just
make ofono_debug_flags a simple enum and then use the bitmask only
internally.
> va_list ap;
>
> - if (debug_enabled == FALSE)
> + if (!debug_enabled || !(debug_flags & flag))
> return;
>
> va_start(ap, format);
> @@ -98,7 +100,7 @@ void __ofono_toggle_debug(void)
> debug_enabled = TRUE;
> }
>
> -int __ofono_log_init(gboolean detach, gboolean debug)
> +int __ofono_log_init(gboolean detach, gboolean debug, guint dflags)
> {
> int option = LOG_NDELAY | LOG_PID;
>
> @@ -110,6 +112,7 @@ int __ofono_log_init(gboolean detach, gboolean debug)
> syslog(LOG_INFO, "oFono version %s", VERSION);
>
> debug_enabled = debug;
> + debug_flags = dflags;
This is double. If dflags are not set, then don't enable debug. No need
to provide two parameters that do the same.
Also dflags is a pretty weird variable name.
> return 0;
> }
> diff --git a/src/main.c b/src/main.c
> index 7542e13..7227bde 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -54,12 +54,33 @@ static void system_bus_disconnected(DBusConnection *conn,
> void *user_data)
>
> static gboolean option_detach = TRUE;
> static gboolean option_debug = FALSE;
> +static guint debug_flags = 0;
> +
> +static GDebugKey keys[] = {
> + { "core", OFONO_DEBUG_CORE },
> +};
> +
> +static gboolean parse_debug_flags(const gchar *option_name, const gchar
> *value,
> + gpointer data, GError **err)
> +{
> + option_debug = TRUE;
> +
> + /* NULL means no string was supplied to --debug. We default to "core"
> + * in that scenario; perhaps we should be defaulting to "all" instead?
> */
> + if (!value)
> + value = "core";
> +
> + debug_flags = g_parse_debug_string(value, keys,
> + sizeof(keys) / sizeof(keys[0]));
> + return TRUE;
> +}
>
> static GOptionEntry options[] = {
> { "nodetach", 'n', G_OPTION_FLAG_REVERSE,
> G_OPTION_ARG_NONE, &option_detach,
> "Don't run as daemon in background" },
> - { "debug", 'd', 0, G_OPTION_ARG_NONE, &option_debug,
> + { "debug", 'd', G_OPTION_FLAG_OPTIONAL_ARG,
> + G_OPTION_ARG_CALLBACK, &parse_debug_flags,
> "Enable debug information output" },
>
