You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by as...@apache.org on 2021/11/04 17:23:59 UTC
[qpid-proton] 05/16: PROTON-2449: Refactor logging/error reporting
to avoid scratch string
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 <as...@apache.org>
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: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org