From 0b231195cb5be7d0c92b5d8164b222a4e0b9f287 Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Tue, 15 Mar 2016 22:15:02 +0100 Subject: [PATCH] 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. --- 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 171d2b1e15..78d8627738 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 cfa44376f5..591d38e413 100644 --- 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)) + 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; - 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 { - goto cleanup; - } - virSkipSpaces(&cur); + + count++; } + ret = count; cleanup: - if (ret == -1) - VIR_WARN("Ignoring invalid log output setting."); + virStringFreeList(strings); return ret; }