diff mbox series

[v5,11/13] log: add a per line log helper

Message ID 20231220153607.718606-12-david.marchand@redhat.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series Detect superfluous newline in logs |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

David Marchand Dec. 20, 2023, 3:36 p.m. UTC 
  gcc builtin __builtin_strchr can be used as a static assertion to check
whether passed format strings contain a \n.
This can be useful to detect double \n in log messages.

Signed-off-by: David Marchand <david.marchand@redhat.com>
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
---
Changes since v4:
- fixed build with -pedantic,

Changes since v3:
- fixed some checkpatch complaints,

Changes since RFC v1:
- added a check in checkpatches.sh,

---
 devtools/checkpatches.sh |  8 ++++++++
 lib/log/rte_log.h        | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+)

Comments

David Marchand Dec. 20, 2023, 3:42 p.m. UTC | #1 
On Wed, Dec 20, 2023 at 4:39 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> gcc builtin __builtin_strchr can be used as a static assertion to check
> whether passed format strings contain a \n.
> This can be useful to detect double \n in log messages.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Acked-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
> Changes since v4:
> - fixed build with -pedantic,

Unfortunately, upon testing, clang does not support constant
expression folding in (Ubuntu 20.04 and Fedora 37 at least) older
versions.
So we may manage to make this check work with clang, but that's for the future.
diff mbox series

Patch

diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 10b79ca2bc..10d1bf490b 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -53,6 +53,14 @@  print_usage () {
 check_forbidden_additions() { # <patch>
 	res=0
 
+	# refrain from new calls to RTE_LOG
+	awk -v FOLDERS="lib" \
+		-v EXPRESSIONS="RTE_LOG\\\(" \
+		-v RET_ON_FAIL=1 \
+		-v MESSAGE='Prefer RTE_LOG_LINE' \
+		-f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \
+		"$1" || res=1
+
 	# refrain from new additions of rte_panic() and rte_exit()
 	# multiple folders and expressions are separated by spaces
 	awk -v FOLDERS="lib drivers" \
diff --git a/lib/log/rte_log.h b/lib/log/rte_log.h
index 3394746103..5ba198ba24 100644
--- a/lib/log/rte_log.h
+++ b/lib/log/rte_log.h
@@ -17,6 +17,7 @@ 
 extern "C" {
 #endif
 
+#include <assert.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdarg.h>
@@ -358,6 +359,26 @@  int rte_vlog(uint32_t level, uint32_t logtype, const char *format, va_list ap)
 		 RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) :	\
 	 0)
 
+#if defined(RTE_TOOLCHAIN_GCC) && !defined(PEDANTIC)
+#define RTE_LOG_CHECK_NO_NEWLINE(fmt) \
+	static_assert(!__builtin_strchr(fmt, '\n'), \
+		"This log format string contains a \\n")
+#else
+#define RTE_LOG_CHECK_NO_NEWLINE(...)
+#endif
+
+#define RTE_LOG_LINE(l, t, ...) do { \
+	RTE_LOG_CHECK_NO_NEWLINE(RTE_FMT_HEAD(__VA_ARGS__ ,)); \
+	RTE_LOG(l, t, RTE_FMT(RTE_FMT_HEAD(__VA_ARGS__ ,) "\n", \
+		RTE_FMT_TAIL(__VA_ARGS__ ,))); \
+} while (0)
+
+#define RTE_LOG_DP_LINE(l, t, ...) do { \
+	RTE_LOG_CHECK_NO_NEWLINE(RTE_FMT_HEAD(__VA_ARGS__ ,)); \
+	RTE_LOG_DP(l, t, RTE_FMT(RTE_FMT_HEAD(__VA_ARGS__ ,) "\n", \
+		RTE_FMT_TAIL(__VA_ARGS__ ,))); \
+} while (0)
+
 #define RTE_LOG_REGISTER_IMPL(type, name, level)			    \
 int type;								    \
 RTE_INIT(__##type)							    \