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" },
{ 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);
Same as from above. If debug_flags = 0, then debug is disabled.
Regards
Marcel