[libvirt] [PATCH 20/38] virlog: Split output parsing and output defining to separate operations

Erik Skultety eskultet at redhat.com
Thu Mar 31 17:48:53 UTC 2016


Everything has been prepared to successfully split parsing and defining logic
to separate operations.
---
 src/util/virlog.c  | 160 +++++++++++++++++++++++++++++------------------------
 src/util/virlog.h  |  15 +++--
 tests/testutils.c  |  19 ++++++-
 tests/virlogtest.c |   5 +-
 4 files changed, 114 insertions(+), 85 deletions(-)

diff --git a/src/util/virlog.c b/src/util/virlog.c
index 7e0936c..369d7dd 100644
--- a/src/util/virlog.c
+++ b/src/util/virlog.c
@@ -343,64 +343,50 @@ virLogOutputFree(virLogOutputPtr output)
  * @c: the function to call to close the output (or NULL)
  * @data: extra data passed as first arg to the function
  * @priority: minimal priority for this filter, use 0 for none
- * @dest: where to send output of this priority
+ * @dest: where to send output of this priority (see virLogDestination)
  * @name: optional name data associated with an output
- * @flags: extra flag, currently unused
  *
  * Defines an output function for log messages. Each message once
- * gone though filtering is emitted through each registered output.
+ * gone through filtering is emitted through each registered output.
  *
- * Returns -1 in case of failure or the output number if successful
+ * Returns reference to a newly created object or NULL in case of failure.
  */
-int
+virLogOutputPtr
 virLogOutputNew(virLogOutputFunc f,
                 virLogCloseFunc c,
                 void *data,
                 virLogPriority priority,
                 virLogDestination dest,
-                const char *name,
-                unsigned int flags)
+                const char *name)
 {
+    virLogOutputPtr ret = NULL;
     char *ndup = NULL;
-    virLogOutputPtr output = NULL;
-
-    virCheckFlags(0, -1);
-
-    if (virLogInitialize() < 0)
-        return -1;
 
-    if (f == NULL)
-        return -1;
+    if (!f)
+        return NULL;
 
     if (dest == VIR_LOG_TO_SYSLOG || dest == VIR_LOG_TO_FILE) {
-        if (!name) {
-            virReportOOMError();
-            return -1;
-        }
+        if (!name)
+            return NULL;
+
         if (VIR_STRDUP(ndup, name) < 0)
-            return -1;
+            return NULL;
     }
 
-    if (VIR_ALLOC_QUIET(output) < 0) {
+    if (VIR_ALLOC_QUIET(ret) < 0) {
         VIR_FREE(ndup);
-        return -1;
+        return NULL;
     }
 
-    output->logInitMessage = true;
-    output->f = f;
-    output->c = c;
-    output->data = data;
-    output->priority = priority;
-    output->dest = dest;
-    output->name = ndup;
+    ret->logInitMessage = true;
+    ret->f = f;
+    ret->c = c;
+    ret->data = data;
+    ret->priority = priority;
+    ret->dest = dest;
+    ret->name = ndup;
 
-    virLogLock();
-    if (VIR_APPEND_ELEMENT_QUIET(virLogOutputs, virLogNbOutputs, output))
-        goto cleanup;
-
- cleanup:
-    virLogUnlock();
-    return virLogNbOutputs;
+    return ret;
 }
 
 
@@ -717,32 +703,32 @@ virLogCloseFd(void *data)
 }
 
 
-static int
+static virLogOutputPtr
 virLogNewOutputToStderr(virLogPriority priority)
 {
-    if (virLogOutputNew(virLogOutputToFd, NULL, (void *)2L, priority,
-                        VIR_LOG_TO_STDERR, NULL, 0) < 0)
-        return -1;
-    return 0;
+    return virLogOutputNew(virLogOutputToFd, NULL, (void *)2L, priority,
+                           VIR_LOG_TO_STDERR, NULL);
 }
 
 
-static int
+static virLogOutputPtr
 virLogNewOutputToFile(virLogPriority priority,
                       const char *file)
 {
     int fd;
+    virLogOutputPtr ret = NULL;
 
     fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR);
     if (fd < 0)
-        return -1;
-    if (virLogOutputNew(virLogOutputToFd, virLogCloseFd,
-                        (void *)(intptr_t)fd,
-                        priority, VIR_LOG_TO_FILE, file, 0) < 0) {
-        VIR_FORCE_CLOSE(fd);
-        return -1;
+        return NULL;
+
+    if (!(ret = virLogOutputNew(virLogOutputToFd, virLogCloseFd,
+                                (void *)(intptr_t)fd,
+                                priority, VIR_LOG_TO_FILE, file))) {
+        VIR_LOG_CLOSE(fd);
+        return NULL;
     }
-    return 0;
+    return ret;
 }
 
 
@@ -811,25 +797,28 @@ virLogCloseSyslog(void *data ATTRIBUTE_UNUSED)
 }
 
 
-static int
+static virLogOutputPtr
 virLogNewOutputToSyslog(virLogPriority priority,
                         const char *ident)
 {
+    virLogOutputPtr ret = NULL;
+
     /*
      * ident needs to be kept around on Solaris
      */
     VIR_FREE(current_ident);
     if (VIR_STRDUP(current_ident, ident) < 0)
-        return -1;
+        return NULL;
 
     openlog(current_ident, 0, 0);
-    if (virLogOutputNew(virLogOutputToSyslog, virLogCloseSyslog, NULL,
-                        priority, VIR_LOG_TO_SYSLOG, ident, 0) < 0) {
+    if (!(ret = virLogOutputNew(virLogOutputToSyslog, virLogCloseSyslog,
+                                NULL, priority, VIR_LOG_TO_SYSLOG,
+                                ident))) {
         closelog();
         VIR_FREE(current_ident);
-        return -1;
+        return NULL;
     }
-    return 0;
+    return ret;
 }
 
 
@@ -1035,19 +1024,25 @@ static void virLogCloseJournald(void *data ATTRIBUTE_UNUSED)
 }
 
 
-static int virLogNewOutputToJournald(int priority)
+static virLogOutputPtr virLogNewOutputToJournald(int priority)
 {
+    virLogOutputPtr ret = NULL;
+
     if ((journalfd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0)
-        return -1;
+        return NULL;
     if (virSetInherit(journalfd, false) < 0) {
         VIR_LOG_CLOSE(journalfd);
-        return -1;
+        return NULL;
     }
-    if (virLogOutputNew(virLogOutputToJournald, virLogCloseJournald, NULL,
-                        priority, VIR_LOG_TO_JOURNALD, NULL, 0) < 0) {
-        return -1;
+
+    if (!(ret = virLogOutputNew(virLogOutputToJournald,
+                                virLogCloseJournald, NULL,
+                                priority, VIR_LOG_TO_JOURNALD, NULL))) {
+        VIR_LOG_CLOSE(journalfd);
+        return NULL;
     }
-    return 0;
+
+    return ret;
 }
 # endif /* USE_JOURNALD */
 
@@ -1081,11 +1076,10 @@ int virLogPriorityFromSyslog(int priority ATTRIBUTE_UNUSED)
     ((*cur == ' ') || (*cur == '\t') || (*cur == '\n') ||               \
      (*cur == '\r') || (*cur == '\\'))
 
-
-static int
+static virLogOutputPtr
 virLogParseOutput(const char *src)
 {
-    int ret = -1;
+    virLogOutputPtr ret = NULL;
     char **tokens = NULL;
     char *abspath = NULL;
     size_t count = 0;
@@ -1094,7 +1088,7 @@ virLogParseOutput(const char *src)
     bool isSUID = virIsSUID();
 
     if (!src)
-        return -1;
+        return NULL;
 
     VIR_DEBUG("output=%s", src);
 
@@ -1102,7 +1096,7 @@ virLogParseOutput(const char *src)
      * them individually
      */
     if (!(tokens = virStringSplitCount(src, ":", 0, &count)))
-        return -1;
+        return NULL;
 
     if (virStrToLong_uip(tokens[0], NULL, 10, &prio) < 0 ||
         (prio < VIR_LOG_DEBUG) || (prio > VIR_LOG_ERROR))
@@ -1146,7 +1140,7 @@ virLogParseOutput(const char *src)
     }
 
  cleanup:
-    if (ret < 0)
+    if (!ret)
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to parse and define log output %s"), src);
     virStringFreeList(tokens);
@@ -1180,12 +1174,14 @@ virLogParseOutput(const char *src)
  * Returns the number of output parsed or -1 in case of error.
  */
 int
-virLogParseOutputs(const char *src)
+virLogParseOutputs(const char *src, virLogOutputPtr **outputs)
 {
     int ret = -1;
-    int count = 0;
+    size_t count = 0;
     size_t i;
     char **strings = NULL;
+    virLogOutputPtr output = NULL;
+    virLogOutputPtr *list = NULL;
 
     if (!src)
         return -1;
@@ -1200,14 +1196,22 @@ virLogParseOutputs(const char *src)
         if (STREQ(strings[i], ""))
             continue;
 
-        if (virLogParseOutput(strings[i]) < 0)
+        if (!(output = virLogParseOutput(strings[i])))
             goto cleanup;
 
-        count++;
+        if (VIR_APPEND_ELEMENT(list, count, output) < 0) {
+            virLogOutputFree(output);
+            goto cleanup;
+        }
+
+        virLogOutputFree(output);
     }
 
     ret = count;
+    *outputs = list;
  cleanup:
+    if (ret < 0)
+        virLogOutputListFree(list, count);
     virStringFreeList(strings);
     return ret;
 }
@@ -1535,12 +1539,22 @@ int
 virLogSetOutputs(const char *outputs)
 {
     int ret = -1;
+    int count = 0;
+    virLogOutputPtr *list = NULL;
 
     if (virLogInitialize() < 0)
         return -1;
 
-    ret = virLogParseOutputs(outputs);
+    if ((count = virLogParseOutputs(outputs, &list)) < 0)
+        goto cleanup;
 
+    if (virLogDefineOutputs(list, count) < 0)
+        goto cleanup;
+
+    ret = count;
+ cleanup:
+    if (ret < 0)
+        virLogOutputListFree(list, count);
     return ret;
 }
 
diff --git a/src/util/virlog.h b/src/util/virlog.h
index eca7894..9b9f643 100644
--- a/src/util/virlog.h
+++ b/src/util/virlog.h
@@ -188,13 +188,12 @@ extern int virLogSetOutputs(const char *outputs);
 extern int virLogFilterNew(const char *match,
                            virLogPriority priority,
                            unsigned int flags);
-extern int virLogOutputNew(virLogOutputFunc f,
-                           virLogCloseFunc c,
-                           void *data,
-                           virLogPriority priority,
-                           virLogDestination dest,
-                           const char *name,
-                           unsigned int flags);
+extern virLogOutputPtr virLogOutputNew(virLogOutputFunc f,
+                                       virLogCloseFunc c,
+                                       void *data,
+                                       virLogPriority priority,
+                                       virLogDestination dest,
+                                       const char *name);
 extern void virLogOutputFree(virLogOutputPtr output);
 extern void virLogFilterFree(virLogFilterPtr filter);
 extern void virLogOutputListFree(virLogOutputPtr *list, int count);
@@ -209,7 +208,7 @@ extern void virLogUnlock(void);
 extern int virLogReset(void);
 extern int virLogParseDefaultPriority(const char *priority);
 extern int virLogParseFilters(const char *filters);
-extern int virLogParseOutputs(const char *output);
+extern int virLogParseOutputs(const char *src, virLogOutputPtr **outputs);
 extern int virLogPriorityFromSyslog(int priority);
 extern void virLogMessage(virLogSourcePtr source,
                           virLogPriority priority,
diff --git a/tests/testutils.c b/tests/testutils.c
index 4998931..e8da060 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -838,6 +838,9 @@ int virtTestMain(int argc,
 {
     int ret;
     char *testRange = NULL;
+    size_t count = 0;
+    virLogOutputPtr output = NULL;
+    virLogOutputPtr *list = NULL;
 #ifdef TEST_OOM
     char *oomstr;
 #endif
@@ -869,9 +872,21 @@ int virtTestMain(int argc,
 
     virLogSetFromEnv();
     if (!getenv("LIBVIRT_DEBUG") && !virLogGetNbOutputs()) {
-        if (virLogOutputNew(virtTestLogOutput, virtTestLogClose, &testLog,
-                            VIR_LOG_DEBUG, VIR_LOG_TO_STDERR, NULL, 0) < 0)
+        if (!(output = virLogOutputNew(virtTestLogOutput, virtTestLogClose,
+                                       &testLog, VIR_LOG_DEBUG,
+                                       VIR_LOG_TO_STDERR, NULL)))
             return EXIT_FAILURE;
+
+        if (VIR_APPEND_ELEMENT(list, count, output) < 0) {
+            virLogOutputFree(output);
+            return EXIT_FAILURE;
+        }
+
+        if (virLogDefineOutputs(list, count) < 0) {
+            virLogOutputListFree(list, count);
+            return EXIT_FAILURE;
+        }
+
     }
 
     if ((testRange = getenv("VIR_TEST_RANGE")) != NULL) {
diff --git a/tests/virlogtest.c b/tests/virlogtest.c
index 6abd4ae..02613f5 100644
--- a/tests/virlogtest.c
+++ b/tests/virlogtest.c
@@ -48,9 +48,10 @@ testLogParseOutputs(const void *opaque)
 {
     int ret = -1;
     int noutputs;
+    virLogOutputPtr *list = NULL;
     const struct testLogData *data = opaque;
 
-    noutputs = virLogParseOutputs(data->str);
+    noutputs = virLogParseOutputs(data->str, &list);
     if (noutputs < 0) {
         if (!data->pass) {
             VIR_TEST_DEBUG("Got expected error: %s\n",
@@ -70,7 +71,7 @@ testLogParseOutputs(const void *opaque)
 
     ret = 0;
  cleanup:
-    virLogReset();
+    virLogOutputListFree(list, noutputs);
     return ret;
 }
 
-- 
2.4.3




More information about the libvir-list mailing list