This is an automated email from the ASF dual-hosted git repository. astitcher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
commit cf72c6158029b4466d8eec75d3ae7bdd94109aef Author: Andrew Stitcher <[email protected]> AuthorDate: Fri Jun 4 17:50:05 2021 -0400 PROTON-2449: Refactor logging/error reporting to avoid scratch string There is already a scratch string in the logger so having another one in transport for the purpose of formatting error messages is unnecessary. --- c/src/core/connection_driver.c | 8 ++------ c/src/core/dispatcher.c | 16 +++++++--------- c/src/core/engine-internal.h | 6 ------ c/src/core/logger.c | 23 ++++++++++++++++++++++- c/src/core/logger_private.h | 8 ++++++++ c/src/core/transport.c | 2 -- 6 files changed, 39 insertions(+), 24 deletions(-) diff --git a/c/src/core/connection_driver.c b/c/src/core/connection_driver.c index 1305a12..803917c 100644 --- a/c/src/core/connection_driver.c +++ b/c/src/core/connection_driver.c @@ -43,9 +43,7 @@ static pn_event_t *batch_next(pn_connection_driver_t *d) { /* Log the next event that will be processed */ pn_event_t *next = pn_collector_next(d->collector); if (next && PN_SHOULD_LOG(&d->transport->logger, PN_SUBSYSTEM_EVENT, PN_LEVEL_DEBUG)) { - pn_string_clear(d->transport->scratch); - pn_inspect(next, d->transport->scratch); - pni_logger_log(&d->transport->logger, PN_SUBSYSTEM_EVENT, PN_LEVEL_DEBUG, pn_string_get(d->transport->scratch)); + pni_logger_log_msg_inspect(&d->transport->logger, PN_SUBSYSTEM_EVENT, PN_LEVEL_DEBUG, next, ""); } return next; } @@ -151,9 +149,7 @@ bool pn_connection_driver_finished(pn_connection_driver_t *d) { void pn_connection_driver_verrorf(pn_connection_driver_t *d, const char *name, const char *fmt, va_list ap) { pn_transport_t *t = d->transport; pn_condition_t *cond = pn_transport_condition(t); - pn_string_vformat(t->scratch, fmt, ap); - pn_condition_set_name(cond, name); - pn_condition_set_description(cond, pn_string_get(t->scratch)); + pn_condition_vformat(cond, name, fmt, ap); } void pn_connection_driver_errorf(pn_connection_driver_t *d, const char *name, const char *fmt, ...) { diff --git a/c/src/core/dispatcher.c b/c/src/core/dispatcher.c index 4df8b17..7df9ade 100644 --- a/c/src/core/dispatcher.c +++ b/c/src/core/dispatcher.c @@ -75,7 +75,8 @@ static inline int pni_dispatch_action(pn_transport_t* transport, uint64_t lcode, return action(transport, frame_type, channel, args, payload); } -static int pni_dispatch_frame(pn_transport_t * transport, pn_data_t *args, pn_frame_t frame) + +static int pni_dispatch_frame(pn_frame_t frame, pn_logger_t *logger, pn_transport_t * transport, pn_data_t *args) { pn_bytes_t frame_payload = frame.frame_payload0; @@ -84,11 +85,8 @@ static int pni_dispatch_frame(pn_transport_t * transport, pn_data_t *args, pn_fr } ssize_t dsize = pn_data_decode(args, frame_payload.start, frame_payload.size); if (dsize < 0) { - pn_string_format(transport->scratch, - "Error decoding frame: %s %s\n", pn_code(dsize), - pn_error_text(pn_data_error(args))); - pn_quote(transport->scratch, frame_payload.start, frame_payload.size); - PN_LOG(&transport->logger, PN_SUBSYSTEM_AMQP, PN_LEVEL_ERROR, pn_string_get(transport->scratch)); + PN_LOG_MSG_DATA(logger, PN_SUBSYSTEM_AMQP, PN_LEVEL_ERROR, frame_payload, + "Error decoding frame: %s %s\n", pn_code(dsize), pn_error_text(pn_data_error(args))); return dsize; } @@ -100,11 +98,11 @@ static int pni_dispatch_frame(pn_transport_t * transport, pn_data_t *args, pn_fr bool scanned; int e = pn_data_scan(args, "D?L.", &scanned, &lcode); if (e) { - PN_LOG(&transport->logger, PN_SUBSYSTEM_AMQP, PN_LEVEL_ERROR, "Scan error"); + PN_LOG(logger, PN_SUBSYSTEM_AMQP, PN_LEVEL_ERROR, "Scan error"); return e; } if (!scanned) { - PN_LOG(&transport->logger, PN_SUBSYSTEM_AMQP, PN_LEVEL_ERROR, "Error dispatching frame"); + PN_LOG(logger, PN_SUBSYSTEM_AMQP, PN_LEVEL_ERROR, "Error dispatching frame"); return PN_ERR; } size_t payload_size = frame_payload.size - dsize; @@ -130,7 +128,7 @@ ssize_t pn_dispatcher_input(pn_transport_t *transport, const char *bytes, size_t read += n; available -= n; transport->input_frames_ct += 1; - int e = pni_dispatch_frame(transport, transport->args, frame); + int e = pni_dispatch_frame(frame, &transport->logger, transport, transport->args); if (e) return e; } else if (n < 0) { pn_do_error(transport, "amqp:connection:framing-error", "malformed frame"); diff --git a/c/src/core/engine-internal.h b/c/src/core/engine-internal.h index 832d29d..471f597 100644 --- a/c/src/core/engine-internal.h +++ b/c/src/core/engine-internal.h @@ -165,7 +165,6 @@ struct pn_transport_t { /* scratch area */ - pn_string_t *scratch; pn_data_t *args; pn_data_t *output_args; pn_buffer_t *frame; // frame under construction @@ -381,11 +380,6 @@ void pn_ep_decref(pn_endpoint_t *endpoint); int pn_post_frame(pn_transport_t *transport, uint8_t type, uint16_t ch, const char *fmt, ...); -typedef enum {IN, OUT} pn_dir_t; - -void pn_do_trace(pn_transport_t *transport, uint16_t ch, pn_dir_t dir, - pn_data_t *args, const char *payload, size_t size); - #if __cplusplus } #endif diff --git a/c/src/core/logger.c b/c/src/core/logger.c index b697c3d..4f9df7e 100644 --- a/c/src/core/logger.c +++ b/c/src/core/logger.c @@ -17,10 +17,11 @@ * under the License. */ +#include "logger_private.h" + #include <proton/logger.h> #include <proton/error.h> -#include "logger_private.h" #include "memory.h" #include "util.h" #include "value_dump.h" @@ -206,6 +207,26 @@ void pni_logger_log_raw(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_lo pni_logger_log(logger, subsystem, severity, pn_string_get(logger->scratch)); } +void pni_logger_log_msg_data(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, pn_bytes_t data, const char *fmt, ...) { + va_list ap; + + va_start(ap, fmt); + pn_string_vformat(logger->scratch, fmt, ap); + va_end(ap); + pn_quote(logger->scratch, data.start, data.size); + pni_logger_log(logger, subsystem, severity, pn_string_get(logger->scratch)); +} + +void pni_logger_log_msg_inspect(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, void* object, const char *fmt, ...) { + va_list ap; + + va_start(ap, fmt); + pn_string_vformat(logger->scratch, fmt, ap); + va_end(ap); + pn_inspect(object, logger->scratch); + pni_logger_log(logger, subsystem, severity, pn_string_get(logger->scratch)); +} + void pni_logger_log_msg_frame(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, pn_bytes_t frame, const char *fmt, ...) { va_list ap; diff --git a/c/src/core/logger_private.h b/c/src/core/logger_private.h index 818e4c1..e6e3f6f 100644 --- a/c/src/core/logger_private.h +++ b/c/src/core/logger_private.h @@ -44,7 +44,9 @@ void pni_logger_fini(pn_logger_t*); void pni_logger_log(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, const char *message); void pni_logger_vlogf(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, const char *fmt, va_list ap); void pni_logger_log_data(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, const char *msg, const char *bytes, size_t size); +void pni_logger_log_msg_data(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, pn_bytes_t data, const char *fmt, ...); void pni_logger_log_raw(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, pn_buffer_t *output, size_t size); +void pni_logger_log_msg_inspect(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, void *object, const char *fmt, ...); void pni_logger_log_msg_frame(pn_logger_t *logger, pn_log_subsystem_t subsystem, pn_log_level_t severity, pn_bytes_t frame, const char *fmt, ...); #define PN_SHOULD_LOG(logger, subsys, sev) \ @@ -64,6 +66,12 @@ void pni_logger_log_msg_frame(pn_logger_t *logger, pn_log_subsystem_t subsystem, pni_logger_log_data(logger, (pn_log_subsystem_t) (subsys), (pn_log_level_t) (sev), __VA_ARGS__); \ } while(0) +#define PN_LOG_MSG_DATA(logger, subsys, sev, data, ...) \ + do { \ + if (PN_SHOULD_LOG(logger, subsys, sev)) \ + pni_logger_log_msg_data(logger, (pn_log_subsystem_t) (subsys), (pn_log_level_t) (sev), data, __VA_ARGS__); \ + } while(0) + #define PN_LOG_RAW(logger, subsys, sev, ...) \ do { \ if (PN_SHOULD_LOG(logger, subsys, sev)) \ diff --git a/c/src/core/transport.c b/c/src/core/transport.c index 63c7a1a..fb1e02e 100644 --- a/c/src/core/transport.c +++ b/c/src/core/transport.c @@ -407,7 +407,6 @@ static void pn_transport_initialize(void *object) transport->sasl = NULL; transport->ssl = NULL; - transport->scratch = pn_string(NULL); transport->args = pn_data(16); transport->output_args = pn_data(16); transport->frame = pn_buffer(PN_TRANSPORT_INITIAL_FRAME_SIZE); @@ -663,7 +662,6 @@ static void pn_transport_finalize(void *object) pn_free(transport->remote_channels); pni_mem_subdeallocate(pn_class(transport), transport, transport->input_buf); pni_mem_subdeallocate(pn_class(transport), transport, transport->output_buf); - pn_free(transport->scratch); pn_data_free(transport->args); pn_data_free(transport->output_args); pn_buffer_free(transport->frame); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
