[pulseaudio-discuss] [PATCH 1/2] Log: Set log target improvement

Shuai Fan shuai900217 at 126.com
Mon May 20 08:49:49 PDT 2013


---
 src/daemon/daemon-conf.c    |  75 +++++----------------
 src/daemon/daemon-conf.h    |   3 +-
 src/daemon/main.c           |  11 +--
 src/pulsecore/cli-command.c |  49 +++++++-------
 src/pulsecore/log.c         | 161 ++++++++++++++++++++++++++++++++++++++++++--
 src/pulsecore/log.h         |  22 ++++--
 6 files changed, 221 insertions(+), 100 deletions(-)

diff --git a/src/daemon/daemon-conf.c b/src/daemon/daemon-conf.c
index f1e5476..7726c90 100644
--- a/src/daemon/daemon-conf.c
+++ b/src/daemon/daemon-conf.c
@@ -73,12 +73,11 @@ static const pa_daemon_conf default_conf = {
     .flat_volumes = TRUE,
     .exit_idle_time = 20,
     .scache_idle_time = 20,
-    .auto_log_target = 1,
     .script_commands = NULL,
     .dl_search_path = NULL,
     .load_default_script_file = TRUE,
     .default_script_file = NULL,
-    .log_target = PA_LOG_SYSLOG,
+    .log_target = NULL,
     .log_level = PA_LOG_NOTICE,
     .log_backtrace = 0,
     .log_meta = FALSE,
@@ -172,6 +171,7 @@ void pa_daemon_conf_free(pa_daemon_conf *c) {
     pa_xfree(c->script_commands);
     pa_xfree(c->dl_search_path);
     pa_xfree(c->default_script_file);
+    pa_log_target_free(c->log_target);
     pa_xfree(c->config_file);
     pa_xfree(c);
 }
@@ -179,65 +179,21 @@ void pa_daemon_conf_free(pa_daemon_conf *c) {
 #define PA_LOG_MAX_SUFFIX_NUMBER 100
 
 int pa_daemon_conf_set_log_target(pa_daemon_conf *c, const char *string) {
+    pa_log_target *log_target = NULL;
+
     pa_assert(c);
     pa_assert(string);
 
-    if (pa_streq(string, "auto"))
-        c->auto_log_target = 1;
-    else if (pa_streq(string, "syslog")) {
-        c->auto_log_target = 0;
-        c->log_target = PA_LOG_SYSLOG;
-    } else if (pa_streq(string, "stderr")) {
-        c->auto_log_target = 0;
-        c->log_target = PA_LOG_STDERR;
-    } else if (pa_startswith(string, "file:")) {
-        char file_path[512];
-        int log_fd;
-
-        pa_strlcpy(file_path, string + 5, sizeof(file_path));
-
-        /* Open target file with user rights */
-        if ((log_fd = open(file_path, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR | S_IWUSR)) >= 0) {
-             c->auto_log_target = 0;
-             c->log_target = PA_LOG_FD;
-             pa_log_set_fd(log_fd);
-        } else {
-            printf("Failed to open target file %s, error : %s\n", file_path, pa_cstrerror(errno));
-            return -1;
-        }
-    } else if (pa_startswith(string, "newfile:")) {
-        char file_path[512];
-        int log_fd;
-        int version = 0;
-        int left_size;
-        char *p;
-
-        pa_strlcpy(file_path, string + 8, sizeof(file_path));
-
-        left_size = sizeof(file_path) - strlen(file_path);
-        p = file_path + strlen(file_path);
-
-        do {
-            memset(p, 0, left_size);
-
-            if (version > 0)
-                pa_snprintf(p, left_size, ".%d", version);
-        } while (++version <= PA_LOG_MAX_SUFFIX_NUMBER &&
-                 (log_fd = open(file_path, O_RDWR|O_TRUNC|O_CREAT|O_EXCL, S_IRUSR | S_IWUSR)) < 0);
-
-        if (version > PA_LOG_MAX_SUFFIX_NUMBER) {
-            memset(p, 0, left_size);
-            printf("Tried to open target files '%s', '%s.1', '%s.2' ... '%s.%d', but all failed.\n",
-                   file_path, file_path, file_path, file_path, PA_LOG_MAX_SUFFIX_NUMBER - 1);
+    if (!pa_streq(string, "auto")) {
+        log_target = pa_log_parse_target(string);
+
+        if (!log_target) {
+            c->log_target = log_target;
             return -1;
-        } else {
-            printf("Opened target file %s\n", file_path);
-            c->auto_log_target = 0;
-            c->log_target = PA_LOG_FD;
-            pa_log_set_fd(log_fd);
         }
-    } else
-        return -1;
+    }
+
+    c->log_target = log_target;
 
     return 0;
 }
@@ -755,6 +711,7 @@ char *pa_daemon_conf_dump(pa_daemon_conf *c) {
 
     pa_strbuf *s;
     char cm[PA_CHANNEL_MAP_SNPRINT_MAX];
+    char *p;
 
     pa_assert(c);
 
@@ -765,6 +722,8 @@ char *pa_daemon_conf_dump(pa_daemon_conf *c) {
 
     pa_assert(c->log_level < PA_LOG_LEVEL_MAX);
 
+    p = pa_log_target_snprint(c->log_target);
+
     pa_strbuf_printf(s, "daemonize = %s\n", pa_yes_no(c->daemonize));
     pa_strbuf_printf(s, "fail = %s\n", pa_yes_no(c->fail));
     pa_strbuf_printf(s, "high-priority = %s\n", pa_yes_no(c->high_priority));
@@ -787,7 +746,7 @@ char *pa_daemon_conf_dump(pa_daemon_conf *c) {
     pa_strbuf_printf(s, "dl-search-path = %s\n", pa_strempty(c->dl_search_path));
     pa_strbuf_printf(s, "default-script-file = %s\n", pa_strempty(pa_daemon_conf_get_default_script_file(c)));
     pa_strbuf_printf(s, "load-default-script-file = %s\n", pa_yes_no(c->load_default_script_file));
-    pa_strbuf_printf(s, "log-target = %s\n", c->auto_log_target ? "auto" : (c->log_target == PA_LOG_SYSLOG ? "syslog" : "stderr"));
+    pa_strbuf_printf(s, "log-target = %s\n", pa_strempty(p));
     pa_strbuf_printf(s, "log-level = %s\n", log_level_to_string[c->log_level]);
     pa_strbuf_printf(s, "resample-method = %s\n", pa_resample_method_to_string(c->resample_method));
     pa_strbuf_printf(s, "enable-remixing = %s\n", pa_yes_no(!c->disable_remixing));
@@ -846,5 +805,7 @@ char *pa_daemon_conf_dump(pa_daemon_conf *c) {
 #endif
 #endif
 
+    pa_xfree(p);
+
     return pa_strbuf_tostring_free(s);
 }
diff --git a/src/daemon/daemon-conf.h b/src/daemon/daemon-conf.h
index faf2540..067ff21 100644
--- a/src/daemon/daemon-conf.h
+++ b/src/daemon/daemon-conf.h
@@ -80,12 +80,11 @@ typedef struct pa_daemon_conf {
     pa_server_type_t local_server_type;
     int exit_idle_time,
         scache_idle_time,
-        auto_log_target,
         realtime_priority,
         nice_level,
         resample_method;
     char *script_commands, *dl_search_path, *default_script_file;
-    pa_log_target_t log_target;
+    pa_log_target *log_target;
     pa_log_level_t log_level;
     unsigned log_backtrace;
     char *config_file;
diff --git a/src/daemon/main.c b/src/daemon/main.c
index c18524f..0c9ed17 100644
--- a/src/daemon/main.c
+++ b/src/daemon/main.c
@@ -499,8 +499,11 @@ int main(int argc, char *argv[]) {
         goto finish;
     }
 
+    if (!conf->log_target)
+        conf->log_target = pa_log_target_new(PA_LOG_STDERR, NULL);
+
+    pa_log_set_target(conf->log_target);
     pa_log_set_level(conf->log_level);
-    pa_log_set_target(conf->auto_log_target ? PA_LOG_STDERR : conf->log_target);
     if (conf->log_meta)
         pa_log_set_flags(PA_LOG_PRINT_META, PA_LOG_SET);
     if (conf->log_time)
@@ -810,8 +813,8 @@ int main(int argc, char *argv[]) {
         daemon_pipe[0] = -1;
 #endif
 
-        if (conf->auto_log_target)
-            pa_log_set_target(PA_LOG_SYSLOG);
+        if (conf->log_target)
+            pa_log_set_target(conf->log_target);
 
 #ifdef HAVE_SETSID
         if (setsid() < 0) {
@@ -1042,7 +1045,7 @@ int main(int argc, char *argv[]) {
             c->cpu_info.cpu_type = PA_CPU_X86;
         if (pa_cpu_init_arm(&(c->cpu_info.flags.arm)))
             c->cpu_info.cpu_type = PA_CPU_ARM;
-	pa_cpu_init_orc(c->cpu_info);
+        pa_cpu_init_orc(c->cpu_info);
     }
 
     pa_assert_se(pa_signal_init(pa_mainloop_get_api(mainloop)) == 0);
diff --git a/src/pulsecore/cli-command.c b/src/pulsecore/cli-command.c
index f5489d6..3f57101 100644
--- a/src/pulsecore/cli-command.c
+++ b/src/pulsecore/cli-command.c
@@ -188,7 +188,7 @@ static const struct command commands[] = {
     { "kill-client",             pa_cli_command_kill_client,        "Kill a client (args: index)", 2},
     { "kill-sink-input",         pa_cli_command_kill_sink_input,    "Kill a sink input (args: index)", 2},
     { "kill-source-output",      pa_cli_command_kill_source_output, "Kill a source output (args: index)", 2},
-    { "set-log-target",          pa_cli_command_log_target,         "Change the log target (args: null,auto,syslog,stderr,file:PATH)", 2},
+    { "set-log-target",          pa_cli_command_log_target,         "Change the log target (args: null,auto,syslog,stderr,file:PATH,newfile:PATH)", 2},
     { "set-log-level",           pa_cli_command_log_level,          "Change the log level (args: numeric level)", 2},
     { "set-log-meta",            pa_cli_command_log_meta,           "Show source code location in log messages (args: bool)", 2},
     { "set-log-time",            pa_cli_command_log_time,           "Show timestamps in log messages (args: bool)", 2},
@@ -1501,6 +1501,7 @@ static int pa_cli_command_suspend(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, p
 
 static int pa_cli_command_log_target(pa_core *c, pa_tokenizer *t, pa_strbuf *buf, pa_bool_t *fail) {
     const char *m;
+    pa_log_target *log_target = NULL;
 
     pa_core_assert_ref(c);
     pa_assert(t);
@@ -1508,34 +1509,30 @@ static int pa_cli_command_log_target(pa_core *c, pa_tokenizer *t, pa_strbuf *buf
     pa_assert(fail);
 
     if (!(m = pa_tokenizer_get(t, 1))) {
-        pa_strbuf_puts(buf, "You need to specify a log target (null,auto,syslog,stderr,file:PATH).\n");
-        return -1;
-    }
-
-    if (pa_streq(m, "null"))
-        pa_log_set_target(PA_LOG_NULL);
-    else if (pa_streq(m, "syslog"))
-        pa_log_set_target(PA_LOG_SYSLOG);
-    else if (pa_streq(m, "stderr") || pa_streq(m, "auto")) {
-        /* 'auto' is actually the effect with 'stderr' */
-        pa_log_set_target(PA_LOG_STDERR);
-    } else if (pa_startswith(m, "file:")) {
-        const char *file_path = m + 5;
-        int log_fd;
-
-        /* Open target file with user rights */
-        if ((log_fd = open(file_path, O_RDWR|O_TRUNC|O_CREAT, S_IRUSR | S_IWUSR)) >= 0) {
-            pa_log_set_target(PA_LOG_FD);
-            pa_log_set_fd(log_fd);
-        } else {
-            pa_strbuf_printf(buf, "Failed to open target file %s, error : %s\n", file_path, pa_cstrerror(errno));
-            return -1;
-        }
-    } else {
-        pa_strbuf_puts(buf, "You need to specify a log target (null,auto,syslog,stderr,file:PATH).\n");
+        pa_strbuf_puts(buf, "You need to specify a log target (null,auto,syslog,stderr,file:PATH,newfile:PATH).\n");
         return -1;
     }
 
+    log_target = pa_log_parse_target(m);
+
+    switch (log_target->type) {
+        case PA_LOG_STDERR:
+        case PA_LOG_SYSLOG:
+        case PA_LOG_NULL:
+            pa_log_set_target(log_target);
+            break;
+        case PA_LOG_FILE:
+        case PA_LOG_NEWFILE:
+            if (pa_log_set_target(log_target) < 0) {
+                pa_strbuf_puts(buf, "Failed to set log target.");
+                return -1;
+            }
+            break;
+        default:
+            pa_strbuf_puts(buf, "You need to specify a log target (null,auto,syslog,stderr,file:PATH,newfile:PATH).\n");
+            return -1;
+    }
+
     return 0;
 }
 
diff --git a/src/pulsecore/log.c b/src/pulsecore/log.c
index 8eaef54..f95031a 100644
--- a/src/pulsecore/log.c
+++ b/src/pulsecore/log.c
@@ -29,6 +29,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
+#include <fcntl.h>
 
 #ifdef HAVE_EXECINFO_H
 #include <execinfo.h>
@@ -47,9 +48,11 @@
 
 #include <pulsecore/macro.h>
 #include <pulsecore/core-util.h>
+#include <pulsecore/core-error.h>
 #include <pulsecore/once.h>
 #include <pulsecore/ratelimit.h>
 #include <pulsecore/thread.h>
+#include <pulsecore/i18n.h>
 
 #include "log.h"
 
@@ -63,9 +66,11 @@
 #define ENV_LOG_BACKTRACE "PULSE_LOG_BACKTRACE"
 #define ENV_LOG_BACKTRACE_SKIP "PULSE_LOG_BACKTRACE_SKIP"
 #define ENV_LOG_NO_RATELIMIT "PULSE_LOG_NO_RATE_LIMIT"
+#define LOG_MAX_SUFFIX_NUMBER 100
 
 static char *ident = NULL; /* in local charset format */
-static pa_log_target_t target = PA_LOG_STDERR, target_override;
+static pa_log_target target = { PA_LOG_STDERR, NULL };
+static pa_log_target_type_t target_override;
 static pa_bool_t target_override_set = FALSE;
 static pa_log_level_t maximum_level = PA_LOG_ERROR, maximum_level_override = PA_LOG_ERROR;
 static unsigned show_backtrace = 0, show_backtrace_override = 0, skip_backtrace = 0;
@@ -113,10 +118,62 @@ void pa_log_set_level(pa_log_level_t l) {
     maximum_level = l;
 }
 
-void pa_log_set_target(pa_log_target_t t) {
-    pa_assert(t < PA_LOG_TARGET_MAX);
+int pa_log_set_target(pa_log_target *t) {
+    char *file_path = NULL;
+    int length = 0;
+    int fd = -1;
+    int version = 0;
+    int left_size = 0;
+    char *p = NULL;
+
+    pa_assert(t);
+
+    switch (t->type) {
+        case PA_LOG_STDERR:
+        case PA_LOG_SYSLOG:
+        case PA_LOG_NULL:
+            target = *t;
+            break;
+        case PA_LOG_FILE:
+            if ((fd = pa_open_cloexec(t->file, O_WRONLY | O_TRUNC | O_CREAT, S_IRUSR | S_IWUSR)) >= 0) {
+                target = *t;
+                pa_log_set_fd(fd);
+            } else {
+                pa_log(_("Failed to open target file '%s'."), t->file);
+                return -1;
+            }
+            break;
+        case PA_LOG_NEWFILE:
+            left_size = LOG_MAX_SUFFIX_NUMBER / 10 + 2;
+            length = strlen(t->file) + left_size;
+            file_path = pa_xmalloc(length);
+            pa_strlcpy(file_path, t->file, length);
+            p = file_path + strlen(t->file);
+
+            do {
+                memset(p, 0, left_size);
+
+                if (version > 0)
+                    pa_snprintf(p, left_size, ".%d", version);
+            } while (++version < LOG_MAX_SUFFIX_NUMBER &&
+                    (fd = pa_open_cloexec(file_path, O_WRONLY | O_TRUNC | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR)) < 0);
+
+            if (version > LOG_MAX_SUFFIX_NUMBER) {
+                memset(p, 0, left_size);
+                pa_log(_("Tried to open target file '%s', '%s.1', '%s.2' ... '%s.%d', but all failed."),
+                        file_path, file_path, file_path, file_path, LOG_MAX_SUFFIX_NUMBER - 1);
+                return -1;
+            } else {
+                pa_log_debug("Opened target file %s\n", file_path);
+                target = *t;
+                pa_log_set_fd(fd);
+            }
+            break;
+        default:
+            return -1;
+    }
 
-    target = t;
+    return 0;
 }
 
 void pa_log_set_flags(pa_log_flags_t _flags, pa_log_merge_t merge) {
@@ -276,7 +333,7 @@ void pa_log_levelv_meta(
     char *t, *n;
     int saved_errno = errno;
     char *bt = NULL;
-    pa_log_target_t _target;
+    pa_log_target_type_t _target;
     pa_log_level_t _maximum_level;
     unsigned _show_backtrace;
     pa_log_flags_t _flags;
@@ -290,7 +347,7 @@ void pa_log_levelv_meta(
 
     init_defaults();
 
-    _target = target_override_set ? target_override : target;
+    _target = target_override_set ? target_override : target.type;
     _maximum_level = PA_MAX(maximum_level, maximum_level_override);
     _show_backtrace = PA_MAX(show_backtrace, show_backtrace_override);
     _flags = flags | flags_override;
@@ -410,7 +467,8 @@ void pa_log_levelv_meta(
             }
 #endif
 
-            case PA_LOG_FD: {
+            case PA_LOG_FILE:
+            case PA_LOG_NEWFILE: {
                 if (log_fd >= 0) {
                     char metadata[256];
 
@@ -473,3 +531,92 @@ pa_bool_t pa_log_ratelimit(pa_log_level_t level) {
 
     return pa_ratelimit_test(&ratelimit, level);
 }
+
+pa_log_target *pa_log_target_new(pa_log_target_type_t type, char *file) {
+    pa_log_target *t = NULL;
+
+    t = pa_xnew(pa_log_target, 1);
+
+    t->type = type;
+    t->file = file;
+
+    return t;
+}
+
+void pa_log_target_free(pa_log_target *t) {
+    pa_assert(t);
+
+    pa_xfree(t->file);
+    pa_xfree(t);
+}
+
+pa_log_target *pa_log_parse_target(const char *string) {
+    pa_log_target *t = NULL;
+
+    pa_assert(string);
+
+    if (pa_streq(string, "stderr"))
+        t = pa_log_target_new(PA_LOG_STDERR, NULL);
+    else if (pa_streq(string, "syslog"))
+        t = pa_log_target_new(PA_LOG_SYSLOG, NULL);
+    else if (pa_streq(string, "null"))
+        t = pa_log_target_new(PA_LOG_NULL, NULL);
+    else if (pa_startswith(string, "file:")) {
+        char *file = NULL;
+        int length = 0;
+
+        length = strlen(string) - 5;
+        file = pa_xmalloc(length + 1);
+        pa_strlcpy(file, string + 5, length + 1);
+
+        t = pa_log_target_new(PA_LOG_FILE, file);
+    } else if (pa_startswith(string, "newfile:")) {
+        char *file = NULL;
+        int length = 0;
+
+        length = strlen(string) - 8;
+        file = pa_xmalloc(length + 1);
+        pa_strlcpy(file, string + 8, length + 1);
+
+        t = pa_log_target_new(PA_LOG_NEWFILE, file);
+    } else
+        pa_log(_("Invalid log target."));
+
+    return t;
+}
+
+char *pa_log_target_snprint(const pa_log_target *t) {
+    char *string = NULL;
+    int length = 0;
+
+    pa_assert(t);
+
+    switch (t->type) {
+        case PA_LOG_STDERR:
+            length = strlen("stderr") + 1;
+            string = pa_xmalloc(length);
+            pa_strlcpy(string, "stderr", length);
+            break;
+        case PA_LOG_SYSLOG:
+            length = strlen("syslog") + 1;
+            string = pa_xmalloc(length);
+            pa_strlcpy(string, "syslog", length);
+            break;
+        case PA_LOG_NULL:
+            length = strlen("null") + 1;
+            string = pa_xmalloc(length);
+            pa_strlcpy(string, "null", length);
+            break;
+        case PA_LOG_FILE:
+        case PA_LOG_NEWFILE:
+            length = strlen(t->file) + 1;
+            string = pa_xmalloc(length);
+            pa_strlcpy(string, t->file, length);
+            break;
+        default:
+            printf("Invalid log target\n");
+            break;
+    }
+
+    return string;
+}
diff --git a/src/pulsecore/log.h b/src/pulsecore/log.h
index 8dd056b..a52acb8 100644
--- a/src/pulsecore/log.h
+++ b/src/pulsecore/log.h
@@ -32,13 +32,14 @@
 /* A simple logging subsystem */
 
 /* Where to log to */
-typedef enum pa_log_target {
+typedef enum pa_log_target_type {
     PA_LOG_STDERR,  /* default */
     PA_LOG_SYSLOG,
     PA_LOG_NULL,    /* to /dev/null */
-    PA_LOG_FD,      /* to a file descriptor, e.g. a char device */
+    PA_LOG_FILE,      /* to a user specified file */
+    PA_LOG_NEWFILE,     /* with an automatic suffix to avoid overwriting anything */
     PA_LOG_TARGET_MAX
-} pa_log_target_t;
+} pa_log_target_type_t;
 
 typedef enum pa_log_level {
     PA_LOG_ERROR  = 0,    /* Error messages */
@@ -63,11 +64,16 @@ typedef enum pa_log_merge {
     PA_LOG_RESET
 } pa_log_merge_t;
 
+typedef struct {
+    pa_log_target_type_t type;
+    char *file;
+} pa_log_target;
+
 /* Set an identification for the current daemon. Used when logging to syslog. */
 void pa_log_set_ident(const char *p);
 
 /* Set a log target. */
-void pa_log_set_target(pa_log_target_t t);
+int pa_log_set_target(pa_log_target *t);
 
 /* Maximal log level */
 void pa_log_set_level(pa_log_level_t l);
@@ -109,6 +115,14 @@ void pa_log_levelv(
         const char *format,
         va_list ap);
 
+pa_log_target *pa_log_target_new(pa_log_target_type_t type, char *file);
+
+void pa_log_target_free(pa_log_target *t);
+
+pa_log_target *pa_log_parse_target(const char *string);
+
+char *pa_log_target_snprint(const pa_log_target *t);
+
 #if __STDC_VERSION__ >= 199901L
 
 /* ISO varargs available */
-- 
1.8.1.3




More information about the pulseaudio-discuss mailing list