You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by gm...@apache.org on 2016/03/03 18:55:14 UTC

qpid-dispatch git commit: DISPATCH-225 - Modified qd_log_entity() and log_sink_lh() to not crash if the specified output log file is invalid. Create instance of log_sink_t only when file can be created

Repository: qpid-dispatch
Updated Branches:
  refs/heads/master e239c9d4a -> abd6b5ce2


DISPATCH-225 - Modified qd_log_entity() and log_sink_lh() to not crash if the specified output log file is invalid. Create instance of log_sink_t only when file can be created


Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo
Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/abd6b5ce
Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/abd6b5ce
Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/abd6b5ce

Branch: refs/heads/master
Commit: abd6b5ce27e222e068543ec42478db02d0fe5cd5
Parents: e239c9d
Author: Ganesh Murthy <gm...@redhat.com>
Authored: Thu Mar 3 09:32:18 2016 -0500
Committer: Ganesh Murthy <gm...@redhat.com>
Committed: Thu Mar 3 10:31:45 2016 -0500

----------------------------------------------------------------------
 include/qpid/dispatch/error.h  |   1 +
 src/log.c                      | 132 +++++++++++++++++++++---------------
 tests/system_tests_qdmanage.py |  16 +++++
 3 files changed, 95 insertions(+), 54 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/abd6b5ce/include/qpid/dispatch/error.h
----------------------------------------------------------------------
diff --git a/include/qpid/dispatch/error.h b/include/qpid/dispatch/error.h
index e269e5e..6bf7e82 100644
--- a/include/qpid/dispatch/error.h
+++ b/include/qpid/dispatch/error.h
@@ -96,6 +96,7 @@ extern const int QD_ERROR_MAX;
 qd_error_t qd_error_py_impl(const char *file, int line);
 
 #define QD_ERROR_RET() do { if (qd_error_code()) return qd_error_code(); } while(0)
+#define QD_ERROR_BREAK() if (qd_error_code()) break;
 #define QD_ERROR_PY_RET() do { if (qd_error_py()) return qd_error_code(); } while(0)
 
 /**

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/abd6b5ce/src/log.c
----------------------------------------------------------------------
diff --git a/src/log.c b/src/log.c
index a867d74..f2657e4 100644
--- a/src/log.c
+++ b/src/log.c
@@ -90,47 +90,60 @@ static log_sink_t* find_log_sink_lh(const char* name) {
     return sink;
 }
 
+// Must hold the log_source_lock
+static void log_sink_free_lh(log_sink_t* sink) {
+    if (!sink) return;
+    assert(sink->refcount);
+
+    if (--sink->refcount == 0) {
+        DEQ_REMOVE(sink_list, sink);
+        free(sink->name);
+        if (sink->file && sink->file != stderr)
+            fclose(sink->file);
+        if (sink->syslog)
+            closelog();
+        free(sink);
+    }
+}
+
 static log_sink_t* log_sink_lh(const char* name) {
     log_sink_t* sink = find_log_sink_lh(name);
     if (sink)
         sink->refcount++;
     else {
-        sink = NEW(log_sink_t);
-        *sink = (log_sink_t){ 1, strdup(name), };
+
+        bool syslog = false;
+        FILE *file = 0;
+
         if (strcmp(name, SINK_STDERR) == 0) {
-            sink->file = stderr;
+            file = stderr;
         }
         else if (strcmp(name, SINK_SYSLOG) == 0) {
             openlog(0, 0, LOG_DAEMON);
-            sink->syslog = true;
+            syslog = true;
         }
         else {
-            sink->file = fopen(name, "w");
-            if (!sink->file) {
-                char msg[TEXT_MAX];
-                snprintf(msg, sizeof(msg), "Failed to open log file '%s'", name);
-                perror(msg);
-                exit(1);
-            }
+            file = fopen(name, "w");
+        }
+
+        //If file is not there, log an error and return 0.
+        if (!file && !syslog) {
+            char msg[TEXT_MAX];
+            snprintf(msg, sizeof(msg), "Failed to open log file '%s'", name);
+            qd_error(QD_ERROR_CONFIG, msg);
+            return 0;
         }
+
+        sink = NEW(log_sink_t);
+        *sink = (log_sink_t){ 1, strdup(name), };
+        sink->syslog = syslog;
+        sink->file = file;
         DEQ_INSERT_TAIL(sink_list, sink);
+
     }
     return sink;
 }
 
-// Must hold the log_source_lock
-static void log_sink_free_lh(log_sink_t* sink) {
-    if (!sink) return;
-    assert(sink->refcount);
-    if (--sink->refcount == 0) {
-        DEQ_REMOVE(sink_list, sink);
-        free(sink->name);
-        if (sink->file && sink->file != stderr)
-            fclose(sink->file);
-        if (sink->syslog) closelog();
-        free(sink);
-    }
-}
 
 struct qd_log_source_t {
     DEQ_LINKS(qd_log_source_t);
@@ -435,39 +448,50 @@ void qd_log_finalize(void) {
 qd_error_t qd_log_entity(qd_entity_t *entity) {
 
     qd_error_clear();
-    char* module = qd_entity_get_string(entity, "module"); QD_ERROR_RET();
+
+    //Obtain the log_source_lock global lock
     sys_mutex_lock(log_source_lock);
-    qd_log_source_t *src = qd_log_source_lh(module); /* The original log source */
-    free(module);
-    qd_log_source_t copy = *src; /* A copy to modify outside the lock. */
-    sys_mutex_unlock(log_source_lock);
 
-    if (qd_entity_has(entity, "enable")) {
-        char *enable = qd_entity_get_string(entity, "enable");
-        copy.mask = enable_mask(enable);
-        free(enable);
-    }
-    QD_ERROR_RET();
-
-    if (qd_entity_has(entity, "timestamp"))
-        copy.timestamp = qd_entity_get_bool(entity, "timestamp");
-    QD_ERROR_RET();
-
-    if (qd_entity_has(entity, "source"))
-        copy.source = qd_entity_get_bool(entity, "source");
-    QD_ERROR_RET();
-
-    if (qd_entity_has(entity, "output")) {
-         log_sink_free_lh(copy.sink); /* DEFAULT source may already have a sink */
-        char* output = qd_entity_get_string(entity, "output"); QD_ERROR_RET();
-        copy.sink = log_sink_lh(output);
-        free(output);
-        if (copy.sink->syslog) /* Timestamp off for syslog. */
-            copy.timestamp = 0;
-    }
+    do {
+
+        char* module = qd_entity_get_string(entity, "module");
+        QD_ERROR_BREAK();
+
+        qd_log_source_t *src = qd_log_source_lh(module); /* The original(already existing) log source */
+        free(module);
+
+        if (qd_entity_has(entity, "output")) {
+            char* output = qd_entity_get_string(entity, "output");
+            QD_ERROR_BREAK();
+            log_sink_t* sink = log_sink_lh(output);
+            free(output);
+            QD_ERROR_BREAK();
+
+            log_sink_free_lh(src->sink); /* DEFAULT source may already have a sink, so free that sink first */
+            src->sink = sink;           /* Assign the new sink   */
+
+            if (src->sink->syslog) /* Timestamp off for syslog. */
+                src->timestamp = 0;
+        }
+
+        if (qd_entity_has(entity, "enable")) {
+            char *enable = qd_entity_get_string(entity, "enable");
+            src->mask = enable_mask(enable);
+            free(enable);
+        }
+        QD_ERROR_BREAK();
+
+        if (qd_entity_has(entity, "timestamp"))
+            src->timestamp = qd_entity_get_bool(entity, "timestamp");
+        QD_ERROR_BREAK();
+
+        if (qd_entity_has(entity, "source"))
+            src->source = qd_entity_get_bool(entity, "source");
+        QD_ERROR_BREAK();
+
+    } while(0);
 
-    sys_mutex_lock(log_source_lock);
-    *src = copy;
     sys_mutex_unlock(log_source_lock);
+
     return qd_error_code();
 }

http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/abd6b5ce/tests/system_tests_qdmanage.py
----------------------------------------------------------------------
diff --git a/tests/system_tests_qdmanage.py b/tests/system_tests_qdmanage.py
index 16f9f18..26a2349 100644
--- a/tests/system_tests_qdmanage.py
+++ b/tests/system_tests_qdmanage.py
@@ -163,5 +163,21 @@ class QdmanageTest(TestCase):
         actual = self.run_qdmanage("GET-JSON-SCHEMA")
         self.assertEquals(schema, dictify(json.loads(actual)))
 
+    def test_update(self):
+        exception = False
+        try:
+            # Try to not set 'output'
+            json.loads(self.run_qdmanage("UPDATE --type org.apache.qpid.dispatch.log --name log/DEFAULT output="))
+        except Exception as e:
+            exception = True
+            self.assertTrue("InternalServerErrorStatus: CError: Configuration: Failed to open log file ''" in e.message)
+        self.assertTrue(exception)
+
+        # Set a valid 'output'
+        output = json.loads(self.run_qdmanage("UPDATE --type org.apache.qpid.dispatch.log --name log/DEFAULT "
+                                              "enable=trace+ output=A.log"))
+        self.assertEqual("A.log", output['output'])
+        self.assertEqual("trace+", output['enable'])
+
 if __name__ == '__main__':
     unittest.main(main_module())


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org