[libvirt] [PATCH 3/5] virlog: Refactor virLogParseOutputs
Erik Skultety
eskultet at redhat.com
Wed Mar 16 11:05:35 UTC 2016
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
More information about the libvir-list
mailing list