PROTON-772: Remove all direct printing to stdout and stderr.

Added a simple log facility in log.h.
Former printfs to stderr, stout now use pn_logf().
Output is enabled by setting envionment variable PN_TRACE_LOG=ON or
by calling: pn_log_init(); pn_log_enable(true);
Default is output to stderr, can provide replacement log function via 
pn_log_logger()


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

Branch: refs/heads/master
Commit: 9a72a30cd2762aaa1920300db8298cad30bf7201
Parents: 2828d33
Author: Alan Conway <[email protected]>
Authored: Thu Dec 11 21:39:56 2014 -0500
Committer: Alan Conway <[email protected]>
Committed: Thu Dec 11 21:39:56 2014 -0500

----------------------------------------------------------------------
 proton-c/CMakeLists.txt            |  1 +
 proton-c/include/proton/log.h      | 77 +++++++++++++++++++++++++++++++++
 proton-c/src/codec/codec.c         |  5 ++-
 proton-c/src/events/event.c        |  2 -
 proton-c/src/log.c                 | 71 ++++++++++++++++++++++++++++++
 proton-c/src/messenger/messenger.c | 22 +++++-----
 proton-c/src/posix/driver.c        | 13 +++---
 proton-c/src/transport/transport.c |  6 ++-
 8 files changed, 173 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/proton-c/CMakeLists.txt b/proton-c/CMakeLists.txt
index 4df2843..93e9895 100644
--- a/proton-c/CMakeLists.txt
+++ b/proton-c/CMakeLists.txt
@@ -281,6 +281,7 @@ set (qpid-proton-core
   src/object/iterator.c
   src/object/record.c
 
+  src/log.c
   src/util.c
   src/url.c
   src/error.c

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/include/proton/log.h
----------------------------------------------------------------------
diff --git a/proton-c/include/proton/log.h b/proton-c/include/proton/log.h
new file mode 100644
index 0000000..81f78f9
--- /dev/null
+++ b/proton-c/include/proton/log.h
@@ -0,0 +1,77 @@
+#ifndef LOG_H
+#define LOG_H
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**@file
+ *
+ * Debug logging for messages that are not associated with a transport.
+ * See pn_transport_trace for transport-related logging.
+ */
+
+#include <proton/import_export.h>
+#include <stdbool.h>
+#include <stdarg.h>
+
+/** Callback for customized logging */
+typedef void (*pn_logger_t)(const char *message);
+
+/** Initialize the logging module, enables logging based on the PN_LOG_TRACE 
envionment variable.
+ *
+ * Must be called before any other pn_log_* functions (e.g. from main()).
+ * Not required unless you use pn_log_enable.
+ */
+PN_EXTERN void pn_log_init(void);
+
+/** Enable/disable logging.
+ *
+ * Note that if pn_log_init() was not called then this is not
+ * thread safe. If called concurrently with calls to other pn_log functions
+ * there is a chance that the envionment variable settings will win.
+ */
+PN_EXTERN void pn_log_enable(bool enabled);
+
+/** Return true if logging is enabled. */
+PN_EXTERN bool pn_log_enabled(void);
+
+/** Log a printf style message */
+#define pn_logf(fmt, ...)                       \
+    do {                                        \
+        if (pn_log_enabled())                   \
+            pn_logf_impl(fmt , ##__VA_ARGS__);  \
+    } while(0)
+
+/** va_list version of pn_logf */
+#define pn_vlogf(fmt, ap)                       \
+    do {                                        \
+        if (pn_log_enabled())                   \
+            pn_vlogf_impl(fmt, ap);             \
+    } while(0)
+
+/** Set the logger. By default a logger that prints to stderr is installed.
+ *@param logger Can be 0 to disable logging regardless of pn_log_enable.
+ */
+PN_EXTERN void pn_log_logger(pn_logger_t logger);
+
+/**@internal*/
+PN_EXTERN void pn_logf_impl(const char* fmt, ...);
+/**@internal*/
+PN_EXTERN void pn_vlogf_impl(const char *fmt, va_list ap);
+
+#endif

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/src/codec/codec.c
----------------------------------------------------------------------
diff --git a/proton-c/src/codec/codec.c b/proton-c/src/codec/codec.c
index afd0c57..d9c38b5 100644
--- a/proton-c/src/codec/codec.c
+++ b/proton-c/src/codec/codec.c
@@ -22,6 +22,7 @@
 #include <proton/object.h>
 #include <proton/codec.h>
 #include <proton/error.h>
+#include <proton/log.h>
 #include <assert.h>
 #include <stdio.h>
 #include <string.h>
@@ -621,7 +622,7 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list 
ap)
           }
           break;
         default:
-          fprintf(stderr, "unrecognized * code: 0x%.2X '%c'\n", code, code);
+          pn_logf("unrecognized * code: 0x%.2X '%c'", code, code);
           return PN_ARG_ERR;
         }
       }
@@ -639,7 +640,7 @@ int pn_data_vfill(pn_data_t *data, const char *fmt, va_list 
ap)
       }
       break;
     default:
-      fprintf(stderr, "unrecognized fill code: 0x%.2X '%c'\n", code, code);
+      pn_logf("unrecognized fill code: 0x%.2X '%c'", code, code);
       return PN_ARG_ERR;
     }
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/src/events/event.c
----------------------------------------------------------------------
diff --git a/proton-c/src/events/event.c b/proton-c/src/events/event.c
index 94c8edf..322b89a 100644
--- a/proton-c/src/events/event.c
+++ b/proton-c/src/events/event.c
@@ -141,8 +141,6 @@ pn_event_t *pn_collector_put(pn_collector_t *collector,
   event->type = type;
   pn_class_incref(clazz, event->context);
 
-  //printf("event %s on %p\n", pn_event_type_name(event->type), 
event->context);
-
   return event;
 }
 

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/src/log.c
----------------------------------------------------------------------
diff --git a/proton-c/src/log.c b/proton-c/src/log.c
new file mode 100644
index 0000000..2506de3
--- /dev/null
+++ b/proton-c/src/log.c
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <proton/log.h>
+#include <proton/object.h>
+#include <stdio.h>
+#include "util.h"
+
+
+static void stderr_logger(const char *message) {
+    fprintf(stderr, "%s\n", message);
+}
+
+static pn_logger_t logger = stderr_logger;
+static int enabled = -1;        /* Undefined, use environment variables. */
+
+void pn_log_init() {
+    enabled = pn_env_bool("PN_TRACE_LOG");
+}
+
+void pn_log_enable(bool enabled) {
+    enabled = enabled;
+}
+
+bool pn_log_enabled() {
+    if (!logger) return false;
+    if (enabled == -1) pn_log_init();
+    return enabled;
+}
+
+void pn_vlogf_impl(const char *fmt, va_list ap) {
+    pn_string_t *msg = pn_string("");
+    pn_string_vformat(msg, fmt, ap);
+    fprintf(stderr, "%s\n", pn_string_get(msg));
+}
+
+/**@internal
+ *
+ * Note: We check pn_log_enabled() in the pn_logf macro *before* calling
+ * pn_logf_impl because evaluating the arguments to that call could have
+ * side-effects with performance impact (e.g. calling functions to construct
+ * complicated messages.) It is important that a disabled log statement results
+ * in nothing more than a call to pn_log_enabled().
+ */
+
+void pn_logf_impl(const char *fmt, ...) {
+  va_list ap;
+  va_start(ap, fmt);
+  pn_vlogf_impl(fmt, ap);
+  va_end(ap);
+}
+
+void pn_log_logger(pn_logger_t new_logger) {
+    logger = new_logger;
+}

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/src/messenger/messenger.c
----------------------------------------------------------------------
diff --git a/proton-c/src/messenger/messenger.c 
b/proton-c/src/messenger/messenger.c
index b4d7fe2..be7c1b1 100644
--- a/proton-c/src/messenger/messenger.c
+++ b/proton-c/src/messenger/messenger.c
@@ -24,6 +24,7 @@
 #include <proton/connection.h>
 #include <proton/delivery.h>
 #include <proton/event.h>
+#include <proton/log.h>
 #include <proton/object.h>
 #include <proton/sasl.h>
 #include <proton/session.h>
@@ -195,7 +196,7 @@ static pn_timestamp_t 
pni_connection_deadline(pn_selectable_t *sel)
 
 static void pn_error_report(const char *pfx, const char *error)
 {
-  fprintf(stderr, "%s ERROR %s\n", pfx, error);
+  pn_logf("%s ERROR %s", pfx, error);
 }
 
 void pni_modified(pn_ctx_t *ctx)
@@ -872,7 +873,6 @@ bool pn_messenger_flow(pn_messenger_t *messenger)
     const int more = pn_min( messenger->credit, batch );
     messenger->distributed += more;
     messenger->credit -= more;
-    //    printf("%s: flowing %i to %p\n", messenger->name, more, (void *) 
ctx->link);
     pn_link_flow(link, more);
     pn_list_add(messenger->credited, link);
     updated = true;
@@ -883,10 +883,10 @@ bool pn_messenger_flow(pn_messenger_t *messenger)
   } else {
     // not enough credit for all links
     if (!messenger->draining) {
-      //      printf("%s: let's drain\n", messenger->name);
+      pn_logf("%s: let's drain", messenger->name);
       if (messenger->next_drain == 0) {
         messenger->next_drain = pn_i_now() + 250;
-        //        printf("%s: initializing next_drain\n", messenger->name);
+        pn_logf("%s: initializing next_drain", messenger->name);
       } else if (messenger->next_drain <= pn_i_now()) {
         // initiate drain, free up at most enough to satisfy blocked
         messenger->next_drain = 0;
@@ -894,7 +894,6 @@ bool pn_messenger_flow(pn_messenger_t *messenger)
         for (size_t i = 0; i < pn_list_size(messenger->credited); i++) {
           pn_link_t *link = (pn_link_t *) pn_list_get(messenger->credited, i);
           if (!pn_link_get_drain(link)) {
-            //            printf("%s: initiating drain from %p\n", 
messenger->name, (void *) ctx->link);
             pn_link_set_drain(link, true);
             needed -= pn_link_remote_credit(link);
             messenger->draining++;
@@ -906,7 +905,7 @@ bool pn_messenger_flow(pn_messenger_t *messenger)
           }
         }
       } else {
-        //        printf("%s: delaying\n", messenger->name);
+        pn_logf("%s: delaying", messenger->name);
       }
     }
   }
@@ -968,7 +967,7 @@ static int pn_transport_config(pn_messenger_t *messenger,
 static void pn_condition_report(const char *pfx, pn_condition_t *condition)
 {
   if (pn_condition_is_redirect(condition)) {
-    fprintf(stderr, "%s NOTICE (%s) redirecting to %s:%i\n",
+    pn_logf("%s NOTICE (%s) redirecting to %s:%i",
             pfx,
             pn_condition_get_name(condition),
             pn_condition_redirect_host(condition),
@@ -1206,7 +1205,6 @@ void pn_messenger_process_flow(pn_messenger_t *messenger, 
pn_event_t *event)
       if (!pn_link_draining(link)) {
         // drain completed!
         int drained = pn_link_drained(link);
-        //          printf("%s: drained %i from %p\n", messenger->name, 
drained, (void *) ctx->link);
         messenger->distributed -= drained;
         messenger->credit += drained;
         pn_link_set_drain(link, false);
@@ -1233,7 +1231,7 @@ void pn_messenger_process_delivery(pn_messenger_t 
*messenger, pn_event_t *event)
   if (pn_delivery_readable(d)) {
     int err = pni_pump_in(messenger, 
pn_terminus_get_address(pn_link_source(link)), link);
     if (err) {
-      fprintf(stderr, "%s\n", pn_error_text(messenger->error));
+      pn_logf("%s", pn_error_text(messenger->error));
     }
   }
 }
@@ -1255,13 +1253,13 @@ int pn_messenger_process_events(pn_messenger_t 
*messenger)
     processed++;
     switch (pn_event_type(event)) {
     case PN_CONNECTION_INIT:
-      //printf("connection created: %p\n", (void *) 
pn_event_connection(event));
+      pn_logf("connection created: %p", (void *) pn_event_connection(event));
       break;
     case PN_SESSION_INIT:
-      //printf("session created: %p\n", (void *) pn_event_session(event));
+      pn_logf("session created: %p", (void *) pn_event_session(event));
       break;
     case PN_LINK_INIT:
-      //printf("link created: %p\n", (void *) pn_event_link(event));
+      pn_logf("link created: %p", (void *) pn_event_link(event));
       break;
     case PN_CONNECTION_REMOTE_OPEN:
     case PN_CONNECTION_REMOTE_CLOSE:

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/src/posix/driver.c
----------------------------------------------------------------------
diff --git a/proton-c/src/posix/driver.c b/proton-c/src/posix/driver.c
index fc583ec..40486c0 100644
--- a/proton-c/src/posix/driver.c
+++ b/proton-c/src/posix/driver.c
@@ -38,6 +38,7 @@
 #include <proton/sasl.h>
 #include <proton/ssl.h>
 #include <proton/object.h>
+#include <proton/log.h>
 #include "util.h"
 #include "platform.h"
 
@@ -139,7 +140,7 @@ pn_listener_t *pn_listener(pn_driver_t *driver, const char 
*host,
     pn_listener_t *l = pn_listener_fd(driver, sock, context);
 
     if (driver->trace & (PN_TRACE_FRM | PN_TRACE_RAW | PN_TRACE_DRV))
-      fprintf(stderr, "Listening on %s:%s\n", host, port);
+      pn_logf("Listening on %s:%s", host, port);
 
     return l;
   }
@@ -204,7 +205,7 @@ pn_connector_t *pn_listener_accept(pn_listener_t *l)
     return NULL;
   } else {
     if (l->driver->trace & (PN_TRACE_FRM | PN_TRACE_RAW | PN_TRACE_DRV))
-      fprintf(stderr, "Accepted from %s\n", name);
+      pn_logf("Accepted from %s", name);
     pn_connector_t *c = pn_connector_fd(l->driver, sock, NULL);
     snprintf(c->name, PN_NAME_MAX, "%s", name);
     c->listener = l;
@@ -271,7 +272,7 @@ pn_connector_t *pn_connector(pn_driver_t *driver, const 
char *host,
   c->sasl = pn_sasl(c->transport);
   snprintf(c->name, PN_NAME_MAX, "%s:%s", host, port);
   if (driver->trace & (PN_TRACE_FRM | PN_TRACE_RAW | PN_TRACE_DRV))
-    fprintf(stderr, "Connected to %s\n", c->name);
+    pn_logf("Connected to %s", c->name);
   return c;
 }
 
@@ -536,7 +537,7 @@ void pn_connector_process(pn_connector_t *c)
 
     if (c->input_done && c->output_done) {
       if (c->trace & (PN_TRACE_FRM | PN_TRACE_RAW | PN_TRACE_DRV)) {
-        fprintf(stderr, "Closed %s\n", c->name);
+        pn_logf("Closed %s", c->name);
       }
       pn_connector_close(c);
     }
@@ -718,7 +719,7 @@ int pn_driver_wait_3(pn_driver_t *d)
           pn_connector_close(c);
       else if (idx && (d->fds[idx].revents & POLLHUP)) {
         if (c->trace & (PN_TRACE_FRM | PN_TRACE_RAW | PN_TRACE_DRV)) {
-          fprintf(stderr, "hangup on connector %s\n", c->name);
+          pn_logf("hangup on connector %s", c->name);
         }
         /* poll() is signalling POLLHUP. to see what happened we need
          * to do an actual recv() to get the error code. But we might
@@ -730,7 +731,7 @@ int pn_driver_wait_3(pn_driver_t *d)
           c->pending_write = true;
       } else if (idx && (d->fds[idx].revents & 
~(POLLIN|POLLOUT|POLLERR|POLLHUP))) {
           if (c->trace & (PN_TRACE_FRM | PN_TRACE_RAW | PN_TRACE_DRV)) {
-            fprintf(stderr, "Unexpected poll events: %04x on %s\n",
+            pn_logf("Unexpected poll events: %04x on %s",
                     d->fds[idx].revents, c->name);
           }
       }

http://git-wip-us.apache.org/repos/asf/qpid-proton/blob/9a72a30c/proton-c/src/transport/transport.c
----------------------------------------------------------------------
diff --git a/proton-c/src/transport/transport.c 
b/proton-c/src/transport/transport.c
index 097d863..8b41984 100644
--- a/proton-c/src/transport/transport.c
+++ b/proton-c/src/transport/transport.c
@@ -19,6 +19,8 @@
  *
  */
 
+#include <proton/log.h>
+
 #include "engine/engine-internal.h"
 #include "framing/framing.h"
 #include "sasl/sasl-internal.h"
@@ -363,6 +365,7 @@ static void pn_transport_initialize(void *object)
     (pn_env_bool("PN_TRACE_DRV") ? PN_TRACE_DRV : PN_TRACE_OFF);
 }
 
+
 pn_session_t *pn_channel_state(pn_transport_t *transport, uint16_t channel)
 {
   return (pn_session_t *) pn_hash_get(transport->remote_channels, channel);
@@ -2159,8 +2162,7 @@ void pn_transport_vlogf(pn_transport_t *transport, const 
char *fmt, va_list ap)
     pn_string_vformat(transport->scratch, fmt, ap);
     pn_transport_log(transport, pn_string_get(transport->scratch));
   } else {
-    vfprintf(stderr, fmt, ap);
-    fprintf(stderr, "\n");
+    pn_vlogf(fmt, ap);
   }
 }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to