virlog: Refactor virLogParseOutputs

The problem with the original virLogParseOutputs method was that the way it
parsed the input, walking the string char by char and using absolute jumps
depending on the virLogDestination type, was rather complicated to read.
This patch utilizes virStringSplit method twice, first time to filter out any
spaces and split the input to individual log outputs and then for each
individual output to tokenize it by to the parts according to our
PRIORITY:DESTINATION?(:DATA) format. Also, to STREQLEN for matching destination
was replaced with virDestinationTypeFromString call.

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 171d2b1e1534900b00f248465bbd9d72911cfa45..78d862773890e9baf89d8190f1b16f2a29a39a44 100644 (file)
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -206,6 +206,7 @@ src/util/viriscsi.c
 src/util/virjson.c
 src/util/virkeyfile.c
 src/util/virlockspace.c
+src/util/virlog.c
 src/util/virnetdev.c
 src/util/virnetdevbandwidth.c
 src/util/virnetdevbridge.c

diff --git a/src/util/virlog.c b/src/util/virlog.c
index cfa44376f5b88e322ab6f713b3bc352d9b0ea5ed..591d38e413f55dc82c162add485d6b163eb8888b 100644 (file)
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1080,6 +1080,78 @@ int virLogPriorityFromSyslog(int priority ATTRIBUTE_UNUSED)
      (*cur == '\r') || (*cur == '\\'))
 
 
+static int
+virLogParseOutput(const char *src)
+{
+    int ret = -1;
+    char **tokens = NULL;
+    char *abspath = NULL;
+    size_t count = 0;
+    virLogPriority prio;
+    virLogDestination dest;
+    bool isSUID = virIsSUID();
+
+    if (!src)
+        return -1;
+
+    VIR_DEBUG("output=%s", src);
+
+    /* split our format prio:destination:additional_data to tokens and parse
+     * them individually
+     */
+    if (!(tokens = virStringSplitCount(src, ":", 0, &count)))
+        return -1;
+
+    if (virStrToLong_uip(tokens[0], NULL, 10, &prio) < 0 ||
+        (prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR))
+        goto cleanup;
+
+    if ((dest = virLogDestinationTypeFromString(tokens[1])) < 0)
+        goto cleanup;
+
+    if (((dest == VIR_LOG_TO_STDERR ||
+          dest == VIR_LOG_TO_JOURNALD) && count != 2) ||
+        ((dest == VIR_LOG_TO_FILE ||
+          dest == VIR_LOG_TO_SYSLOG) && count != 3))
+        goto cleanup;
+
+    /* if running with setuid, only 'stderr' is allowed */
+    if (isSUID && dest != VIR_LOG_TO_STDERR)
+        goto cleanup;
+
+    switch ((virLogDestination) dest) {
+    case VIR_LOG_TO_STDERR:
+        ret = virLogAddOutputToStderr(prio);
+        break;
+    case VIR_LOG_TO_SYSLOG:
+#if HAVE_SYSLOG_H
+        ret = virLogAddOutputToSyslog(prio, tokens[2]);
+#endif
+        break;
+    case VIR_LOG_TO_FILE:
+        if (virFileAbsPath(tokens[2], &abspath) < 0)
+            goto cleanup;
+        ret = virLogAddOutputToFile(prio, abspath);
+        VIR_FREE(abspath);
+        break;
+    case VIR_LOG_TO_JOURNALD:
+#if USE_JOURNALD
+        ret = virLogAddOutputToJournald(prio);
+#endif
+        break;
+    case VIR_LOG_TO_OUTPUT_LAST:
+        break;
+    }
+
+ cleanup:
+    if (ret < 0)
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to parse and define log output %s"), src);
+    virStringFreeList(tokens);
+    return ret;
+}
+
+
 /**
  * virLogParseOutputs:
  * @outputs: string defining a (set of) output(s)
@@ -1103,94 +1175,38 @@ int virLogPriorityFromSyslog(int priority ATTRIBUTE_UNUSED)
  * If running in setuid mode, then only the 'stderr' output will
  * be allowed
  *
- * Returns the number of output parsed and installed or -1 in case of error
+ * Returns the number of output parsed or -1 in case of error.
  */
 int
-virLogParseOutputs(const char *outputs)
+virLogParseOutputs(const char *src)
 {
-    const char *cur = outputs, *str;
-    char *name;
-    char *abspath;
-    virLogPriority prio;
     int ret = -1;
     int count = 0;
-    bool isSUID = virIsSUID();
+    size_t i;
+    char **strings = NULL;
 
-    if (cur == NULL)
+    if (!src)
         return -1;
 
-    VIR_DEBUG("outputs=%s", outputs);
+    VIR_DEBUG("outputs=%s", src);
 
-    virSkipSpaces(&cur);
-    while (*cur != 0) {
-        prio = virParseNumber(&cur);
-        if ((prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR))
-            goto cleanup;
-        if (*cur != ':')
-            goto cleanup;
-        cur++;
-        if (STREQLEN(cur, "stderr", 6)) {
-            cur += 6;
-            if (virLogAddOutputToStderr(prio) == 0)
-                count++;
-        } else if (STREQLEN(cur, "syslog", 6)) {
-            if (isSUID)
-                goto cleanup;
-            cur += 6;
-            if (*cur != ':')
-                goto cleanup;
-            cur++;
-            str = cur;
-            while ((*cur != 0) && (!IS_SPACE(cur)))
-                cur++;
-            if (str == cur)
-                goto cleanup;
-#if HAVE_SYSLOG_H
-            if (VIR_STRNDUP(name, str, cur - str) < 0)
-                goto cleanup;
-            if (virLogAddOutputToSyslog(prio, name) == 0)
-                count++;
-            VIR_FREE(name);
-#endif /* HAVE_SYSLOG_H */
-        } else if (STREQLEN(cur, "file", 4)) {
-            if (isSUID)
-                goto cleanup;
-            cur += 4;
-            if (*cur != ':')
-                goto cleanup;
-            cur++;
-            str = cur;
-            while ((*cur != 0) && (!IS_SPACE(cur)))
-                cur++;
-            if (str == cur)
-                goto cleanup;
-            if (VIR_STRNDUP(name, str, cur - str) < 0)
-                goto cleanup;
-            if (virFileAbsPath(name, &abspath) < 0) {
-                VIR_FREE(name);
-                return -1; /* skip warning here because setting was fine */
-            }
-            if (virLogAddOutputToFile(prio, abspath) == 0)
-                count++;
-            VIR_FREE(name);
-            VIR_FREE(abspath);
-        } else if (STREQLEN(cur, "journald", 8)) {
-            if (isSUID)
-                goto cleanup;
-            cur += 8;
-#if USE_JOURNALD
-            if (virLogAddOutputToJournald(prio) == 0)
-                count++;
-#endif /* USE_JOURNALD */
-        } else {
+    if (!(strings = virStringSplit(src, " ", 0)))
+        goto cleanup;
+
+    for (i = 0; strings[i]; i++) {
+        /* virStringSplit may return empty strings */
+        if (STREQ(strings[i], ""))
+            continue;
+
+        if (virLogParseOutput(strings[i]) < 0)
             goto cleanup;
-        }
-        virSkipSpaces(&cur);
+
+        count++;
     }
+
     ret = count;
  cleanup:
-    if (ret == -1)
-        VIR_WARN("Ignoring invalid log output setting.");
+    virStringFreeList(strings);
     return ret;
 }