You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by jd...@apache.org on 2023/04/04 11:21:01 UTC
[qpid-dispatch] branch main updated: DISPATCH-2355: Implement the `format` attribute for `qd_log_impl` and other such functions to hopefully have compile-time check for format strings (#1630)
This is an automated email from the ASF dual-hosted git repository.
jdanek pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-dispatch.git
The following commit(s) were added to refs/heads/main by this push:
new e9add40d DISPATCH-2355: Implement the `format` attribute for `qd_log_impl` and other such functions to hopefully have compile-time check for format strings (#1630)
e9add40d is described below
commit e9add40d5a4218140f57581f759eb457022e2e60
Author: Jiri Daněk <jd...@redhat.com>
AuthorDate: Tue Apr 4 13:20:54 2023 +0200
DISPATCH-2355: Implement the `format` attribute for `qd_log_impl` and other such functions to hopefully have compile-time check for format strings (#1630)
This is backported code from skupper-router, https://github.com/skupperproject/skupper-router/issues/1011
---
include/qpid/dispatch/error.h | 6 ++-
include/qpid/dispatch/log.h | 8 +--
src/aprintf.h | 2 +-
src/error.c | 2 +-
src/log.c | 2 +-
src/message.c | 2 +-
src/python_embedded.c | 4 +-
src/router_core/core_client_api.c | 63 +++++++++++-----------
.../modules/test_hooks/core_test_hooks.c | 2 +-
src/server.c | 7 +--
src/terminus_private.h | 5 +-
tests/c_unittests/test_terminus.cpp | 14 ++---
12 files changed, 62 insertions(+), 55 deletions(-)
diff --git a/include/qpid/dispatch/error.h b/include/qpid/dispatch/error.h
index 0bbce40a..1b4f928b 100644
--- a/include/qpid/dispatch/error.h
+++ b/include/qpid/dispatch/error.h
@@ -68,7 +68,8 @@ ENUM_DECLARE(qd_error);
*/
#define qd_verror(code, fmt, ap) qd_error_vimpl(code, __FILE__, __LINE__, fmt, ap)
-qd_error_t qd_error_impl(qd_error_t code, const char *file, int line, const char *fmt, ...);
+qd_error_t qd_error_impl(qd_error_t code, const char *file, int line, const char *fmt, ...)
+ __attribute__((format(printf, 4, 5)));
qd_error_t qd_error_vimpl(qd_error_t code, const char *file, int line, const char *fmt, va_list ap);
/**
@@ -117,6 +118,7 @@ qd_error_t qd_error_py_impl(const char *file, int line);
*/
#define qd_error_errno(errnum, ...) qd_error_errno_impl(errnum, __FILE__, __LINE__, __VA_ARGS__)
-qd_error_t qd_error_errno_impl(int errnum, const char *file, int line, const char *fmt, ...);
+qd_error_t qd_error_errno_impl(int errnum, const char *file, int line, const char *fmt, ...)
+ __attribute__((format(printf, 4, 5)));
#endif
diff --git a/include/qpid/dispatch/log.h b/include/qpid/dispatch/log.h
index 62650292..c0c2e273 100644
--- a/include/qpid/dispatch/log.h
+++ b/include/qpid/dispatch/log.h
@@ -44,13 +44,15 @@ qd_log_source_t* qd_log_source(const char *module);
/**@internal*/
bool qd_log_enabled(qd_log_source_t *source, qd_log_level_t level);
/**@internal*/
-void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, ...);
+void qd_log_impl(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, ...)
+ __attribute__((format(printf, 5, 6)));
/**
* Another version of the qd_log_impl function. This function unconditionally writes the the message to the log file.
* It does not check to see if the passed in log level is enabled.
*/
-void qd_log_impl_v1(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, ...);
+void qd_log_impl_v1(qd_log_source_t *source, qd_log_level_t level, const char *file, int line, const char *fmt, ...)
+ __attribute__((format(printf, 5, 6)));
void qd_vlog_impl(qd_log_source_t *source, qd_log_level_t level, bool check_level, const char *file, int line, const char *fmt, va_list ap);
/** Log a message
@@ -80,6 +82,6 @@ void qd_vlog_impl(qd_log_source_t *source, qd_log_level_t level, bool check_leve
/** Maximum length for a log message */
int qd_log_max_len();
-void qd_format_string(char* buf, int buf_size, const char *fmt, ...);
+void qd_format_string(char *buf, int buf_size, const char *fmt, ...) __attribute__((format(printf, 3, 4)));
#endif
diff --git a/src/aprintf.h b/src/aprintf.h
index b259d445..99c424bf 100644
--- a/src/aprintf.h
+++ b/src/aprintf.h
@@ -36,6 +36,6 @@ int vaprintf(char **begin, char *end, const char *format, va_list ap_in);
- n > 0: printing was truncated and would have printed n characters. *begin == end-1
- n < 0: error (return value of vsnprintf) no change to *begin
*/
-int aprintf(char **begin, char *end, const char *format, ...);
+int aprintf(char **begin, char *end, const char *format, ...) __attribute__((format(printf, 3, 4)));
#endif
diff --git a/src/error.c b/src/error.c
index beb4e43b..7998f6b3 100644
--- a/src/error.c
+++ b/src/error.c
@@ -207,7 +207,7 @@ qd_error_t qd_error_errno_impl(int errnum, const char *file, int line, const cha
va_start(arglist, fmt);
vaprintf(&begin, end, fmt, arglist);
va_end(arglist);
- aprintf(&begin, end, ": ", errnum);
+ aprintf(&begin, end, ": ");
char *em = ts.error_message;
if(strerror_r(errnum, begin, end - begin) != 0) {
snprintf(begin, end - begin, "Unknown error %d", errnum);
diff --git a/src/log.c b/src/log.c
index 0bada29f..ab848e1d 100644
--- a/src/log.c
+++ b/src/log.c
@@ -657,7 +657,7 @@ QD_EXPORT qd_error_t qd_log_entity(qd_entity_t *entity)
if (error_in_output) {
char msg[TEXT_MAX];
snprintf(msg, sizeof(msg), "Failed to open log file '%s'", outputFile);
- qd_error(QD_ERROR_CONFIG, msg);
+ qd_error(QD_ERROR_CONFIG, "%s", msg);
}
if (error_in_enable) {
qd_error(QD_ERROR_CONFIG, "'%s' is not a valid log level. Should be one of {%s}.", enable, level_names);
diff --git a/src/message.c b/src/message.c
index 1f9f3d67..8f810b49 100644
--- a/src/message.c
+++ b/src/message.c
@@ -343,7 +343,7 @@ char* qd_message_repr(qd_message_t *msg, char* buffer, size_t len, qd_log_bits f
char *begin = buffer;
char *end = buffer + len - sizeof(REPR_END); /* Save space for ending */
bool first = true;
- aprintf(&begin, end, "Message{", msg);
+ aprintf(&begin, end, "Message{");
print_field(msg, QD_FIELD_MESSAGE_ID, "message-id", flags, &first, &begin, end);
print_field(msg, QD_FIELD_USER_ID, "user-id", flags, &first, &begin, end);
print_field(msg, QD_FIELD_TO, "to", flags, &first, &begin, end);
diff --git a/src/python_embedded.c b/src/python_embedded.c
index 0fd6a45f..a7e40bce 100644
--- a/src/python_embedded.c
+++ b/src/python_embedded.c
@@ -584,10 +584,10 @@ typedef struct {
// Parse an iterator to a python object.
static PyObject *py_iter_parse(qd_iterator_t *iter)
{
- qd_parsed_field_t *parsed=0;
+ qd_parsed_field_t *parsed = 0;
if (iter && (parsed = qd_parse(iter))) {
if (!qd_parse_ok(parsed)) {
- qd_error(QD_ERROR_MESSAGE, qd_parse_error(parsed));
+ qd_error(QD_ERROR_MESSAGE, "%s", qd_parse_error(parsed));
qd_parse_free(parsed);
return 0;
}
diff --git a/src/router_core/core_client_api.c b/src/router_core/core_client_api.c
index c292eedb..e24561f0 100644
--- a/src/router_core/core_client_api.c
+++ b/src/router_core/core_client_api.c
@@ -181,8 +181,8 @@ qdrc_client_t *qdrc_client_CT(qdr_core_t *core,
NULL, // target terminus
&receiver_endpoint,
client);
- qd_log(core->log, QD_LOG_TRACE,
- "New core client created c=%p", client);
+ qd_log(core->log, QD_LOG_TRACE, //
+ "New core client created c=%p", (void *) client);
return client;
}
@@ -221,8 +221,8 @@ void qdrc_client_free_CT(qdrc_client_t *client)
qd_hash_free(client->correlations);
free(client->reply_to);
- qd_log(client->core->log, QD_LOG_TRACE,
- "Core client freed c=%p", client);
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Core client freed c=%p", (void *) client);
free_qdrc_client_t(client);
}
@@ -239,8 +239,8 @@ int qdrc_client_request_CT(qdrc_client_t *client,
qdrc_client_request_done_CT_t done_cb)
{
qd_log(client->core->log, QD_LOG_TRACE,
- "New core client request created c=%p, rc=%p",
- client, request_context);
+ "New core client request created c=%p, rc=%p", (void *) client,
+ request_context);
qdrc_client_request_t *req = new_qdrc_client_request_t();
ZERO(req);
@@ -296,9 +296,9 @@ static void _flush_send_queue_CT(qdrc_client_t *client)
DEQ_REMOVE_HEAD_N(SEND_Q, client->send_queue);
req->on_send_queue = false;
- qd_log(client->core->log, QD_LOG_TRACE,
- "Core client request sent c=%p, rc=%p dlv=%p cid=%s",
- client, req->req_context, req->delivery,
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Core client request sent c=%p, rc=%p dlv=%p cid=%s", //
+ (void *) client, req->req_context, (void *) req->delivery, //
*req->correlation_id ? req->correlation_id : "<none>");
if (!presettled && req->on_ack_cb) {
@@ -363,9 +363,9 @@ static void _free_request_CT(qdrc_client_t *client,
error);
}
- qd_log(client->core->log, QD_LOG_TRACE,
- "Freeing core client request c=%p, rc=%p (%s)",
- client, req->req_context,
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Freeing core client request c=%p, rc=%p (%s)", //
+ (void *) client, req->req_context, //
error ? error : "request complete");
free_qdrc_client_request_t(req);
@@ -397,8 +397,8 @@ static void _sender_second_attach_CT(void *context,
{
qdrc_client_t *client = (qdrc_client_t *)context;
- qd_log(client->core->log, QD_LOG_TRACE,
- "Core client sender 2nd attach c=%p", client);
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Core client sender 2nd attach c=%p", (void *) client);
if (!client->sender_up) {
client->sender_up = true;
@@ -415,8 +415,8 @@ static void _receiver_second_attach_CT(void *context,
{
qdrc_client_t *client = (qdrc_client_t *)context;
- qd_log(client->core->log, QD_LOG_TRACE,
- "Core client receiver 2nd attach c=%p", client);
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Core client receiver 2nd attach c=%p", (void *) client);
if (!client->receiver_up) {
client->receiver_up = true;
@@ -437,9 +437,9 @@ static void _sender_flow_CT(void *context,
qdr_core_t *core = client->core;
client->tx_credit += available_credit;
- qd_log(core->log, QD_LOG_TRACE,
- "Core client sender flow granted c=%p credit=%d d=%s",
- client, client->tx_credit, (drain) ? "T" : "F");
+ qd_log(core->log, QD_LOG_TRACE, //
+ "Core client sender flow granted c=%p credit=%d d=%s", //
+ (void *) client, client->tx_credit, (drain) ? "T" : "F");
if (client->tx_credit > 0) {
_flush_send_queue_CT(client);
}
@@ -464,9 +464,9 @@ static void _sender_update_CT(void *context,
{
qdrc_client_t *client = (qdrc_client_t *)context;
- qd_log(client->core->log, QD_LOG_TRACE,
- "Core client sender update c=%p dlv=%p d=%"PRIx64" %s",
- client, delivery, disposition,
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Core client sender update c=%p dlv=%p d=%" PRIx64 " %s", //
+ (void *) client, (void *) delivery, disposition, //
settled ? "settled" : "unsettled");
if (disposition) {
@@ -497,7 +497,7 @@ static void _sender_update_CT(void *context,
qd_log(client->core->log, QD_LOG_DEBUG,
"Core client could not find request for disposition update"
" client=%p delivery=%p",
- client, delivery);
+ (void *) client, (void *) delivery);
}
}
}
@@ -512,8 +512,8 @@ static void _receiver_transfer_CT(void *client_context,
bool complete = qd_message_receive_complete(message);
qd_log(core->log, QD_LOG_TRACE,
- "Core client received msg c=%p complete=%s",
- client, complete ? "T" : "F");
+ "Core client received msg c=%p complete=%s", //
+ (void *) client, complete ? "T" : "F");
if (complete) {
uint64_t disposition = PN_ACCEPTED;
@@ -527,10 +527,9 @@ static void _receiver_transfer_CT(void *client_context,
qd_hash_retrieve(client->correlations, cid_iter, (void **)&req);
qd_iterator_free(cid_iter);
if (req) {
-
qd_log(core->log, QD_LOG_TRACE,
- "Core client received msg c=%p rc=%p cid=%s",
- client, req->req_context, req->correlation_id);
+ "Core client received msg c=%p rc=%p cid=%s", //
+ (void *) client, req->req_context, req->correlation_id);
qd_hash_remove_by_handle(client->correlations, req->hash_handle);
qd_hash_handle_free(req->hash_handle);
@@ -578,8 +577,8 @@ static void _sender_detached_CT(void *client_context,
{
qdrc_client_t *client = (qdrc_client_t *)client_context;
- qd_log(client->core->log, QD_LOG_TRACE,
- "Core client sender detached c=%p", client);
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Core client sender detached c=%p", (void *) client);
if (client->sender_up) {
client->sender_up = false;
@@ -611,8 +610,8 @@ static void _receiver_detached_CT(void *client_context,
{
qdrc_client_t *client = (qdrc_client_t *)client_context;
- qd_log(client->core->log, QD_LOG_TRACE,
- "Core client receiver detached c=%p", client);
+ qd_log(client->core->log, QD_LOG_TRACE, //
+ "Core client receiver detached c=%p", (void *) client);
if (client->receiver_up) {
client->receiver_up = false;
diff --git a/src/router_core/modules/test_hooks/core_test_hooks.c b/src/router_core/modules/test_hooks/core_test_hooks.c
index d6cba2d4..5a68b6a4 100644
--- a/src/router_core/modules/test_hooks/core_test_hooks.c
+++ b/src/router_core/modules/test_hooks/core_test_hooks.c
@@ -651,7 +651,7 @@ static void qdrc_test_client_api_setup(test_module_t *test_module)
_on_conn_event,
0, 0, 0, tc);
- qd_log(test_module->core->log, QD_LOG_TRACE, "client test registered %p", tc->conn_events);
+ qd_log(test_module->core->log, QD_LOG_TRACE, "client test registered %p", (void *) tc->conn_events);
}
diff --git a/src/server.c b/src/server.c
index 807fe783..c95e43cf 100644
--- a/src/server.c
+++ b/src/server.c
@@ -632,6 +632,8 @@ static void on_accept(pn_event_t *e, qd_listener_t *listener)
/* Log the description, set the transport condition (name, description) close the transport tail. */
+void connect_fail(qd_connection_t *ctx, const char *name, const char *description, ...)
+ __attribute__((format(printf, 3, 4)));
void connect_fail(qd_connection_t *ctx, const char *name, const char *description, ...)
{
va_list ap;
@@ -655,7 +657,6 @@ void connect_fail(qd_connection_t *ctx, const char *name, const char *descriptio
}
}
-
/* Get the host IP address for the remote end */
static void set_rhost_port(qd_connection_t *ctx) {
pn_transport_t *tport = pn_connection_transport(ctx->pn_conn);
@@ -1081,11 +1082,11 @@ static bool handle(qd_server_t *qd_server, pn_event_t *e, pn_connection_t *pn_co
pn_condition_get_name(condition), pn_condition_get_description(condition));
strcpy(ctx->connector->conn_msg, conn_msg);
- qd_log(qd_server->log_source, QD_LOG_INFO, conn_msg);
+ qd_log(qd_server->log_source, QD_LOG_INFO, "%s", conn_msg);
} else {
qd_format_string(conn_msg, 300, "[C%"PRIu64"] Connection to %s failed", ctx->connection_id, config->host_port);
strcpy(ctx->connector->conn_msg, conn_msg);
- qd_log(qd_server->log_source, QD_LOG_INFO, conn_msg);
+ qd_log(qd_server->log_source, QD_LOG_INFO, "%s", conn_msg);
}
} else if (ctx && ctx->listener) { /* Incoming connection */
if (condition && pn_condition_is_set(condition)) {
diff --git a/src/terminus_private.h b/src/terminus_private.h
index 8757ea34..47fb178f 100644
--- a/src/terminus_private.h
+++ b/src/terminus_private.h
@@ -28,7 +28,10 @@
// do pointer & length arithmetic without overflowing the destination buffer in
// qdr_terminus_format()
//
-static inline size_t safe_snprintf(char *str, size_t size, const char *format, ...) {
+static inline size_t safe_snprintf(char *str, size_t size, const char *format, ...)
+ __attribute__((format(printf, 3, 4)));
+size_t safe_snprintf(char *str, size_t size, const char *format, ...)
+{
// max size allowed must be INT_MAX (since vsnprintf returns an int)
if (size == 0 || size > INT_MAX) {
return 0;
diff --git a/tests/c_unittests/test_terminus.cpp b/tests/c_unittests/test_terminus.cpp
index 0e5057b8..494d6fc8 100644
--- a/tests/c_unittests/test_terminus.cpp
+++ b/tests/c_unittests/test_terminus.cpp
@@ -49,37 +49,37 @@ TEST_CASE("test_safe_snprintf") {
SUBCASE("valid_inputs") {
SUBCASE("") {
- len = safe_snprintf(output, LEN + 10, TEST_MESSAGE);
+ len = safe_snprintf(output, LEN + 10, "%s", TEST_MESSAGE);
CHECK(LEN == len);
CHECK(output == TEST_MESSAGE);
}
SUBCASE("") {
- len = safe_snprintf(output, LEN + 1, TEST_MESSAGE);
+ len = safe_snprintf(output, LEN + 1, "%s", TEST_MESSAGE);
CHECK(LEN == len);
CHECK(output == TEST_MESSAGE);
}
SUBCASE("") {
- len = safe_snprintf(output, LEN, TEST_MESSAGE);
+ len = safe_snprintf(output, LEN, "%s", TEST_MESSAGE);
CHECK(LEN - 1 == len);
CHECK(output == "somethin");
}
SUBCASE("") {
- len = safe_snprintf(output, 0, TEST_MESSAGE);
+ len = safe_snprintf(output, 0, "%s", TEST_MESSAGE);
CHECK(0 == len);
}
SUBCASE("") {
output[0] = 'a';
- len = safe_snprintf(output, 1, TEST_MESSAGE);
+ len = safe_snprintf(output, 1, "%s", TEST_MESSAGE);
CHECK(0 == len);
CHECK('\0' == output[0]);
}
SUBCASE("") {
- len = safe_snprintf(output, (int)-1, TEST_MESSAGE);
+ len = safe_snprintf(output, (int) -1, "%s", TEST_MESSAGE);
CHECK(0 == len);
}
}
@@ -90,7 +90,7 @@ TEST_CASE("test_safe_snprintf") {
vsnprintf_stub::rc = -1;
output[0] = 'a';
- len = safe_snprintf(output, LEN + 10, TEST_MESSAGE);
+ len = safe_snprintf(output, LEN + 10, "%s", TEST_MESSAGE);
CHECK(0 == len);
CHECK('\0' == output[0]);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org