[PATCH 3/5] virlog: Refactor virLogParseOutputs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.
Overall, the goal of this patch was to make the $(subj) method more readable,
even though it ended up with slightly more insertions than deletions.
---
 po/POTFILES.in    |   1 +
 src/util/virlog.c | 166 ++++++++++++++++++++++++++++++------------------------
 2 files changed, 92 insertions(+), 75 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 171d2b1..78d8627 100644
--- 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 b717947..dc8ca02 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -1078,6 +1078,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;
+    virLogDestinationType 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 ((virLogDestinationType) 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)
@@ -1101,94 +1173,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;
 }
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]