You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Rafael Schloming <rh...@alum.mit.edu> on 2014/12/12 15:56:10 UTC
Re: [2/2] qpid-proton git commit: PROTON-772: Remove all direct
printing to stdout and stderr.
Hi Alan,
Sorry to pick on you again, but it looks like this commit broke the build
when you have javascript enabled:
/home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token pasting
of
',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
pn_logf_impl(fmt , ##__VA_ARGS__); \
^
/home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token pasting
of
',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
2 errors generated.
Maybe now is a good opportunity to install emscripten? ;-)
--Rafael
On Thu, Dec 11, 2014 at 9:43 PM, <ac...@apache.org> wrote:
>
> 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 <ac...@redhat.com>
> Authored: Thu Dec 11 21:39:56 2014 -0500
> Committer: Alan Conway <ac...@redhat.com>
> 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: commits-unsubscribe@qpid.apache.org
> For additional commands, e-mail: commits-help@qpid.apache.org
>
>
Re: [2/2] qpid-proton git commit: PROTON-772: Remove all direct
printing to stdout and stderr.
Posted by Alan Conway <ac...@redhat.com>.
On Fri, 2014-12-12 at 09:56 -0500, Rafael Schloming wrote:
> Hi Alan,
>
> Sorry to pick on you again, but it looks like this commit broke the build
> when you have javascript enabled:
>
> /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token pasting
> of
> ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> pn_logf_impl(fmt , ##__VA_ARGS__); \
> ^
> /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token pasting
> of
> ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> 2 errors generated.
>
> Maybe now is a good opportunity to install emscripten? ;-)
>
Yes, I think it is. On it.
> --Rafael
>
>
> On Thu, Dec 11, 2014 at 9:43 PM, <ac...@apache.org> wrote:
> >
> > 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 <ac...@redhat.com>
> > Authored: Thu Dec 11 21:39:56 2014 -0500
> > Committer: Alan Conway <ac...@redhat.com>
> > 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: commits-unsubscribe@qpid.apache.org
> > For additional commands, e-mail: commits-help@qpid.apache.org
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [2/2] qpid-proton git commit: PROTON-772: Remove all direct printing
to stdout and stderr.
Posted by Fraser Adams <fr...@blueyonder.co.uk>.
On 15/12/14 16:53, Alan Conway wrote:
> I took out the GNU extension trick (didn't even know it was a GNU
> extension) so it should be more portable now. I guess the javascript
> cross-compiler uses a non-gnu CPP.
Yeah, emscripten is actually an LLVM backend, so actually under the hood
uses Clang as the compiler.
IMHO it's actually quite a good thing to be compiling with that even
aside from the JavaScript awesomeness :-) Clang seems to do a much more
thorough job when it comes to error messages than gcc seems to. I
actually committed a couple of trivial changes at the weekend to address
some warnings that were being flagged and I also then re-enabled -Werror
in the JavaScript CMakeLists.txt, so from now on compiler warnings are
going to fail the build. I personally think that's a good thing.
Frase
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [2/2] qpid-proton git commit: PROTON-772: Remove all direct
printing to stdout and stderr.
Posted by Alan Conway <ac...@redhat.com>.
On Fri, 2014-12-12 at 16:38 -0500, Rafael Schloming wrote:
> It works for me once I added log.c to the javascript build.
I took out the GNU extension trick (didn't even know it was a GNU
extension) so it should be more portable now. I guess the javascript
cross-compiler uses a non-gnu CPP.
>
> --Rafael
>
> On Fri, Dec 12, 2014 at 3:50 PM, Alan Conway <ac...@redhat.com> wrote:
> >
> > On Fri, 2014-12-12 at 09:56 -0500, Rafael Schloming wrote:
> > > Hi Alan,
> > >
> > > Sorry to pick on you again, but it looks like this commit broke the build
> > > when you have javascript enabled:
> > >
> > > /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token
> > pasting
> > > of
> > > ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> > > pn_logf_impl(fmt , ##__VA_ARGS__); \
> > > ^
> > > /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token
> > pasting
> > > of
> > > ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> > > 2 errors generated.
> > >
> > > Maybe now is a good opportunity to install emscripten? ;-)
> > >
> >
> > Got rid of the GNU-C extension of using ", ##" (nice of them to give
> > such a detailed error message :) All standard C99 now.
> >
> > I haven't had time to do a javascript build, but I will do one first
> > thing Monday to make sure all is well.
> >
> > >
> > > On Thu, Dec 11, 2014 at 9:43 PM, <ac...@apache.org> wrote:
> > > >
> > > > 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 <ac...@redhat.com>
> > > > Authored: Thu Dec 11 21:39:56 2014 -0500
> > > > Committer: Alan Conway <ac...@redhat.com>
> > > > 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: commits-unsubscribe@qpid.apache.org
> > > > For additional commands, e-mail: commits-help@qpid.apache.org
> > > >
> > > >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> > For additional commands, e-mail: dev-help@qpid.apache.org
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
Re: [2/2] qpid-proton git commit: PROTON-772: Remove all direct
printing to stdout and stderr.
Posted by Rafael Schloming <rh...@alum.mit.edu>.
It works for me once I added log.c to the javascript build.
--Rafael
On Fri, Dec 12, 2014 at 3:50 PM, Alan Conway <ac...@redhat.com> wrote:
>
> On Fri, 2014-12-12 at 09:56 -0500, Rafael Schloming wrote:
> > Hi Alan,
> >
> > Sorry to pick on you again, but it looks like this commit broke the build
> > when you have javascript enabled:
> >
> > /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token
> pasting
> > of
> > ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> > pn_logf_impl(fmt , ##__VA_ARGS__); \
> > ^
> > /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token
> pasting
> > of
> > ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> > 2 errors generated.
> >
> > Maybe now is a good opportunity to install emscripten? ;-)
> >
>
> Got rid of the GNU-C extension of using ", ##" (nice of them to give
> such a detailed error message :) All standard C99 now.
>
> I haven't had time to do a javascript build, but I will do one first
> thing Monday to make sure all is well.
>
> >
> > On Thu, Dec 11, 2014 at 9:43 PM, <ac...@apache.org> wrote:
> > >
> > > 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 <ac...@redhat.com>
> > > Authored: Thu Dec 11 21:39:56 2014 -0500
> > > Committer: Alan Conway <ac...@redhat.com>
> > > 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: commits-unsubscribe@qpid.apache.org
> > > For additional commands, e-mail: commits-help@qpid.apache.org
> > >
> > >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> For additional commands, e-mail: dev-help@qpid.apache.org
>
>
Re: [2/2] qpid-proton git commit: PROTON-772: Remove all direct
printing to stdout and stderr.
Posted by Alan Conway <ac...@redhat.com>.
On Fri, 2014-12-12 at 09:56 -0500, Rafael Schloming wrote:
> Hi Alan,
>
> Sorry to pick on you again, but it looks like this commit broke the build
> when you have javascript enabled:
>
> /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token pasting
> of
> ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> pn_logf_impl(fmt , ##__VA_ARGS__); \
> ^
> /home/rhs/proton/proton-c/include/proton/log.h:57:32: error: token pasting
> of
> ',' and __VA_ARGS__ is a GNU extension [-Werror,-Wgnu]
> 2 errors generated.
>
> Maybe now is a good opportunity to install emscripten? ;-)
>
Got rid of the GNU-C extension of using ", ##" (nice of them to give
such a detailed error message :) All standard C99 now.
I haven't had time to do a javascript build, but I will do one first
thing Monday to make sure all is well.
>
> On Thu, Dec 11, 2014 at 9:43 PM, <ac...@apache.org> wrote:
> >
> > 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 <ac...@redhat.com>
> > Authored: Thu Dec 11 21:39:56 2014 -0500
> > Committer: Alan Conway <ac...@redhat.com>
> > 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: commits-unsubscribe@qpid.apache.org
> > For additional commands, e-mail: commits-help@qpid.apache.org
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org