Repository: qpid-dispatch Updated Branches: refs/heads/master 05dd0dda5 -> 2d1bd7fb0
NO-JIRA: Replace print to stderr in driver.c with qd_log Use the SERVER log source for all driver error messages. Drop PN_TRACE_* env configuration, proton lib does it automatically. Drop some log messages that are duplicated in server.c Project: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/commit/2d1bd7fb Tree: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/tree/2d1bd7fb Diff: http://git-wip-us.apache.org/repos/asf/qpid-dispatch/diff/2d1bd7fb Branch: refs/heads/master Commit: 2d1bd7fb0948ef4160b3f4e30b048555678c651c Parents: bec2523 Author: Alan Conway <[email protected]> Authored: Wed Nov 23 14:12:05 2016 -0500 Committer: Alan Conway <[email protected]> Committed: Fri Nov 25 11:57:40 2016 -0500 ---------------------------------------------------------------------- include/qpid/dispatch/driver.h | 25 ++-------- include/qpid/dispatch/log.h | 14 ++++++ src/log.c | 14 ++++-- src/posix/driver.c | 97 +++++++------------------------------ src/server.c | 2 +- 5 files changed, 46 insertions(+), 106 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/2d1bd7fb/include/qpid/dispatch/driver.h ---------------------------------------------------------------------- diff --git a/include/qpid/dispatch/driver.h b/include/qpid/dispatch/driver.h index cf8508b..52cbbed 100644 --- a/include/qpid/dispatch/driver.h +++ b/include/qpid/dispatch/driver.h @@ -29,6 +29,8 @@ #include <proton/transport.h> #include <proton/types.h> +typedef struct qd_log_source_t qd_log_source_t; + /** @file * API for the Driver Layer. * @@ -56,9 +58,10 @@ typedef enum { /** Construct a driver * * Call qdpn_driver_free() to release the driver object. + * @param log source to use for log messages, the driver does not have it's own. * @return new driver object, NULL if error */ -qdpn_driver_t *qdpn_driver(void); +qdpn_driver_t *qdpn_driver(qd_log_source_t* log); /** Return the most recent error code. * @@ -83,14 +86,6 @@ int qdpn_driver_errno(qdpn_driver_t *d); */ pn_error_t *qdpn_driver_error(qdpn_driver_t *d); -/** Set the tracing level for the given driver. - * - * @param[in] driver the driver to trace - * @param[in] trace the trace level to use. - * @todo pn_trace_t needs documentation - */ -void qdpn_driver_trace(qdpn_driver_t *driver, pn_trace_t trace); - /** Force qdpn_driver_wait() to return * * @param[in] driver the driver to wake up @@ -170,11 +165,6 @@ qdpn_listener_t *qdpn_listener_head(qdpn_driver_t *driver); */ qdpn_listener_t *qdpn_listener_next(qdpn_listener_t *listener); -/** - * @todo qdpn_listener_trace needs documentation - */ -void qdpn_listener_trace(qdpn_listener_t *listener, pn_trace_t trace); - /** Accept a connection that is pending on the listener. * * @param[in] listener the listener to accept the connection on @@ -253,13 +243,6 @@ qdpn_connector_t *qdpn_connector_head(qdpn_driver_t *driver); */ qdpn_connector_t *qdpn_connector_next(qdpn_connector_t *connector); -/** Set the tracing level for the given connector. - * - * @param[in] connector the connector to trace - * @param[in] trace the trace level to use. - */ -void qdpn_connector_trace(qdpn_connector_t *connector, pn_trace_t trace); - /** Service the given connector. * * Handle any inbound data, outbound data, or timing events pending on http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/2d1bd7fb/include/qpid/dispatch/log.h ---------------------------------------------------------------------- diff --git a/include/qpid/dispatch/log.h b/include/qpid/dispatch/log.h index fac8535..0d69346 100644 --- a/include/qpid/dispatch/log.h +++ b/include/qpid/dispatch/log.h @@ -19,6 +19,7 @@ * under the License. */ #include <stdbool.h> +#include <stdarg.h> /**@file * Sending debug/administrative log messages. @@ -44,6 +45,7 @@ qd_log_source_t* qd_log_source(const char *module); bool qd_log_enabled(qd_log_source_t *source, qd_log_level_t level); /**@internal*/ void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, ...); +void qd_vlog_impl(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, va_list ap); /** Log a message * Note: does not evaluate the format args unless the log message is enabled. @@ -57,6 +59,18 @@ void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file qd_log_impl(source, level, __FILE__, __LINE__, __VA_ARGS__); \ } while(0) +/** Log a message, using a va_list. + * Note: does not evaluate the format args unless the log message is enabled. + * @param source qd_log_source_t* source of log message. + * @param level qd_log_level_t log level of message. + * @param ap va_list argument pack. + */ +#define qd_vlog(source, level, fmt, ap) \ + do { \ + if (qd_log_enabled(source, level)) \ + qd_vlog_impl(source, level, __FILE__, __LINE__, fmt, ap); \ + } while(0) + /** Maximum length for a log message */ int qd_log_max_len(); http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/2d1bd7fb/src/log.c ---------------------------------------------------------------------- diff --git a/src/log.c b/src/log.c index 434115f..074735d 100644 --- a/src/log.c +++ b/src/log.c @@ -380,7 +380,7 @@ bool qd_log_enabled(qd_log_source_t *source, qd_log_level_t level) { return level & mask; } -void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, ...) +void qd_vlog_impl(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, va_list ap) { /*----------------------------------------------- Count this log-event in this log's histogram @@ -403,11 +403,7 @@ void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file entry->file = file ? strdup(file) : 0; entry->line = line; time(&entry->time); - va_list ap; - va_start(ap, fmt); vsnprintf(entry->text, TEXT_MAX, fmt, ap); - va_end(ap); - write_log(source, entry); // Bounded buffer of log entries, keep most recent. @@ -418,6 +414,14 @@ void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file sys_mutex_unlock(log_lock); } +void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, ...) +{ + va_list ap; + va_start(ap, fmt); + qd_vlog_impl(source, level, file, line, fmt, ap); + va_end(ap); +} + static PyObject *inc_none() { Py_INCREF(Py_None); return Py_None; } /// Return the log buffer up to limit as a python list. Called by management agent. http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/2d1bd7fb/src/posix/driver.c ---------------------------------------------------------------------- diff --git a/src/posix/driver.c b/src/posix/driver.c index d7c7eea..cdfdb3b 100644 --- a/src/posix/driver.c +++ b/src/posix/driver.c @@ -43,6 +43,7 @@ #endif #include <qpid/dispatch/driver.h> +#include <qpid/dispatch/error.h> #include <qpid/dispatch/threading.h> #include "alloc.h" #include <proton/error.h> @@ -75,7 +76,6 @@ static inline void ignore_result(int unused_result) { struct qdpn_driver_t { qd_log_source_t *log; - pn_trace_t trace; sys_mutex_t *lock; // @@ -122,7 +122,6 @@ struct qdpn_connector_t { int idx; int fd; int status; - pn_trace_t trace; bool pending_tick; bool pending_read; bool pending_write; @@ -140,12 +139,6 @@ ALLOC_DEFINE(qdpn_connector_t); /* Impls */ -static void pni_fatal(const char *text) -{ - fprintf(stderr, "%s\n", text); - exit(1); -} - pn_timestamp_t pn_i_now(void) { struct timespec now; @@ -154,24 +147,11 @@ pn_timestamp_t pn_i_now(void) #else int cid = CLOCK_MONOTONIC; #endif - if (clock_gettime(cid, &now)) pni_fatal("clock_gettime() failed"); - return ((pn_timestamp_t)now.tv_sec) * 1000 + (now.tv_nsec / 1000000); -} - -static bool pni_eq_nocase(const char *a, const char *b) -{ - while (*b) { - if (tolower(*a++) != tolower(*b++)) - return false; + if (clock_gettime(cid, &now)) { + qd_error_errno(errno, "clock_gettime() failed"); + exit(1); } - return !(*a); -} - -static bool pn_env_bool(const char *name) -{ - char *v = getenv(name); - return v && (pni_eq_nocase(v, "true") || pni_eq_nocase(v, "1") || - pni_eq_nocase(v, "yes") || pni_eq_nocase(v, "on")); + return ((pn_timestamp_t)now.tv_sec) * 1000 + (now.tv_nsec / 1000000); } #define pn_min(X,Y) ((X) > (Y) ? (Y) : (X)) @@ -277,7 +257,7 @@ qdpn_listener_t *qdpn_listener(qdpn_driver_t *driver, hints.ai_socktype = SOCK_STREAM; int code = getaddrinfo(host, port, &hints, &addr); if (code) { - qd_log(driver->log, QD_LOG_ERROR, "getaddrinfo(%s, %s): %s\n", host, port, gai_strerror(code)); + qd_log(driver->log, QD_LOG_ERROR, "getaddrinfo(%s, %s): %s", host, port, gai_strerror(code)); return 0; } @@ -315,10 +295,6 @@ qdpn_listener_t *qdpn_listener(qdpn_driver_t *driver, } qdpn_listener_t *l = qdpn_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); - return l; } @@ -370,11 +346,6 @@ qdpn_listener_t *qdpn_listener_next(qdpn_listener_t *listener) return next; } -void qdpn_listener_trace(qdpn_listener_t *l, pn_trace_t trace) -{ - // XXX -} - void *qdpn_listener_context(qdpn_listener_t *l) { return l ? l->context : NULL; @@ -407,7 +378,7 @@ qdpn_connector_t *qdpn_listener_accept(qdpn_listener_t *l, } else { int code = getnameinfo((struct sockaddr *) &addr, addrlen, hostip, MAX_HOST, serv, MAX_SERV, NI_NUMERICHOST | NI_NUMERICSERV); if (code != 0) { - qd_log(l->driver->log, QD_LOG_ERROR, "getnameinfo: %s\n", gai_strerror(code)); + qd_log(l->driver->log, QD_LOG_ERROR, "getnameinfo: %s", gai_strerror(code)); close(sock); return 0; } else { @@ -424,10 +395,6 @@ qdpn_connector_t *qdpn_listener_accept(qdpn_listener_t *l, *counted = true; } } - - if (l->driver->trace & (PN_TRACE_FRM | PN_TRACE_RAW | PN_TRACE_DRV)) - fprintf(stderr, "Accepted from %s\n", name); - qdpn_connector_t *c = qdpn_connector_fd(l->driver, sock, NULL); snprintf(c->name, PN_NAME_MAX, "%s", name); snprintf(c->hostip, PN_NAME_MAX, "%s", hostip); @@ -441,7 +408,7 @@ void qdpn_listener_close(qdpn_listener_t *l) if (l->closed) return; if (close(l->fd) == -1) - perror("close"); + qdpn_log_errno(l->driver, "close"); l->closed = true; } @@ -522,8 +489,6 @@ qdpn_connector_t *qdpn_connector(qdpn_driver_t *driver, qdpn_connector_t *c = qdpn_connector_fd(driver, sock, context); 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); return c; } @@ -543,7 +508,6 @@ qdpn_connector_t *qdpn_connector_fd(qdpn_driver_t *driver, int fd, void *context c->idx = 0; c->fd = fd; c->status = PN_SEL_RD | PN_SEL_WR; - c->trace = driver->trace; c->closed = false; c->wakeup = 0; c->connection = NULL; @@ -552,9 +516,6 @@ qdpn_connector_t *qdpn_connector_fd(qdpn_driver_t *driver, int fd, void *context c->output_done = false; c->context = context; c->listener = NULL; - - qdpn_connector_trace(c, driver->trace); - qdpn_driver_add_connector(driver, c); return c; } @@ -586,13 +547,6 @@ qdpn_connector_t *qdpn_connector_next(qdpn_connector_t *connector) return next; } -void qdpn_connector_trace(qdpn_connector_t *ctor, pn_trace_t trace) -{ - if (!ctor) return; - ctor->trace = trace; - if (ctor->transport) pn_transport_trace(ctor->transport, trace); -} - pn_transport_t *qdpn_connector_transport(qdpn_connector_t *ctor) { return ctor ? ctor->transport : NULL; @@ -610,7 +564,6 @@ void qdpn_connector_set_connection(qdpn_connector_t *ctor, pn_connection_t *conn pn_class_incref(PN_OBJECT, ctor->connection); pn_transport_bind(ctor->transport, connection); } - if (ctor->transport) pn_transport_trace(ctor->transport, ctor->trace); } pn_connection_t *qdpn_connector_connection(qdpn_connector_t *ctor) @@ -757,7 +710,7 @@ void qdpn_connector_process(qdpn_connector_t *c) ssize_t n = recv(c->fd, pn_transport_tail(transport), capacity, 0); if (n < 0) { if (errno != EAGAIN) { - perror("read"); + qdpn_log_errno(c->driver, "read"); c->status &= ~PN_SEL_RD; c->input_done = true; pn_transport_close_tail( transport ); @@ -805,7 +758,7 @@ void qdpn_connector_process(qdpn_connector_t *c) if (n < 0) { // XXX if (errno != EAGAIN) { - perror("send"); + qdpn_log_errno(c->driver, "send"); c->output_done = true; c->status &= ~PN_SEL_WR; pn_transport_close_head( transport ); @@ -826,9 +779,7 @@ void qdpn_connector_process(qdpn_connector_t *c) // Closed? 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); - } + qd_log(c->driver->log, QD_LOG_TRACE, "Closed %s", c->name); qdpn_connector_close(c); } } @@ -836,13 +787,13 @@ void qdpn_connector_process(qdpn_connector_t *c) // driver -qdpn_driver_t *qdpn_driver() +qdpn_driver_t *qdpn_driver(qd_log_source_t *log) { qdpn_driver_t *d = (qdpn_driver_t *) malloc(sizeof(qdpn_driver_t)); if (!d) return NULL; DEQ_INIT(d->listeners); DEQ_INIT(d->connectors); - d->log = qd_log_source("DRIVER"); + d->log = log; d->lock = sys_mutex(); d->listener_next = NULL; d->connector_next = NULL; @@ -851,14 +802,11 @@ qdpn_driver_t *qdpn_driver() d->fds = NULL; d->nfds = 0; d->efd = 0; - d->trace = ((pn_env_bool("PN_TRACE_RAW") ? PN_TRACE_RAW : PN_TRACE_OFF) | - (pn_env_bool("PN_TRACE_FRM") ? PN_TRACE_FRM : PN_TRACE_OFF) | - (pn_env_bool("PN_TRACE_DRV") ? PN_TRACE_DRV : PN_TRACE_OFF)); d->wakeup = 0; d->efd = eventfd(0, EFD_NONBLOCK); if (d->efd < 0) { - perror("Can't create eventfd"); + qdpn_log_errno(d, "Can't create eventfd"); exit(1); } @@ -871,11 +819,6 @@ qdpn_driver_t *qdpn_driver() return d; } -void qdpn_driver_trace(qdpn_driver_t *d, pn_trace_t trace) -{ - d->trace = trace; -} - void qdpn_driver_free(qdpn_driver_t *d) { if (!d) return; @@ -997,9 +940,7 @@ int qdpn_driver_wait_3(qdpn_driver_t *d) if (idx && d->fds[idx].revents & POLLERR) c->socket_error = true; 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); - } + qd_log(c->driver->log, QD_LOG_TRACE, "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 * be in a state where we're not interested in input, in that @@ -1009,10 +950,8 @@ int qdpn_driver_wait_3(qdpn_driver_t *d) else if (d->fds[idx].events & POLLOUT) 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", - d->fds[idx].revents, c->name); - } + qd_log(c->driver->log, QD_LOG_ERROR, "Unexpected poll events: %04x on %s", + d->fds[idx].revents, c->name); } } c = DEQ_NEXT(c); @@ -1027,7 +966,7 @@ int qdpn_driver_wait_3(qdpn_driver_t *d) DEQ_REMOVE_HEAD(d->connectors); DEQ_INSERT_TAIL(d->connectors, c); } - + d->listener_next = DEQ_HEAD(d->listeners); d->connector_next = DEQ_HEAD(d->connectors); sys_mutex_unlock(d->lock); http://git-wip-us.apache.org/repos/asf/qpid-dispatch/blob/2d1bd7fb/src/server.c ---------------------------------------------------------------------- diff --git a/src/server.c b/src/server.c index 86ebc88..821166e 100644 --- a/src/server.c +++ b/src/server.c @@ -1358,7 +1358,7 @@ qd_server_t *qd_server(qd_dispatch_t *qd, int thread_count, const char *containe qd_server->container_name = container_name; qd_server->sasl_config_path = sasl_config_path; qd_server->sasl_config_name = sasl_config_name; - qd_server->driver = qdpn_driver(); + qd_server->driver = qdpn_driver(qd_server->log_source); qd_server->start_handler = 0; qd_server->conn_handler = 0; qd_server->pn_event_handler = 0; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
