Thread (2 messages) 2 messages, 2 authors, 2021-10-24

Re: [PATCH v2 8/8] staging: vchiq_core: fix quoted strings split across lines

From: Joe Perches <joe@perches.com>
Date: 2021-10-24 23:57:03
Also in: linux-arm-kernel, lkml

On Sun, 2021-10-24 at 18:38 -0300, Gaston Gonzalez wrote:
Quoted strings should not be split across lines. As put it in [1]:
"never break user-visible strings such as printk messages because that
breaks the ability to grep for them."

While at it, fix the alignment of the arguments in the sentence.

Note: this introduce a checkpatch CHECK: line length of 123 exceeds 100
columns, as the line now is:

 vchiq_loud_error("%d: service %d (%c%c%c%c) version mismatch - local (%d, min %d) vs. remote (%d, min %d)",

But now the string is grep-able and the whole function call more
clear.
IMO: All of these should be changed

drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:#define VCHIQ_LOG_PREFIX   KERN_INFO "vchiq: "
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#ifndef vchiq_log_error
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#define vchiq_log_error(cat, fmt, ...) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h- do { if (cat >= VCHIQ_LOG_ERROR) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:         printk(VCHIQ_LOG_PREFIX fmt "\n", ##__VA_ARGS__); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#endif
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#ifndef vchiq_log_warning
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#define vchiq_log_warning(cat, fmt, ...) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h- do { if (cat >= VCHIQ_LOG_WARNING) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:          printk(VCHIQ_LOG_PREFIX fmt "\n", ##__VA_ARGS__); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#endif
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#ifndef vchiq_log_info
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#define vchiq_log_info(cat, fmt, ...) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h- do { if (cat >= VCHIQ_LOG_INFO) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:         printk(VCHIQ_LOG_PREFIX fmt "\n", ##__VA_ARGS__); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#endif
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#ifndef vchiq_log_trace
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#define vchiq_log_trace(cat, fmt, ...) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h- do { if (cat >= VCHIQ_LOG_TRACE) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:         printk(VCHIQ_LOG_PREFIX fmt "\n", ##__VA_ARGS__); } while (0)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#endif
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-#define vchiq_loud_error(...) \
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h- vchiq_log_error(vchiq_core_log_level, "===== " __VA_ARGS__)
drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h-

I suggest using the rather more common vchiq_err, vchiq_warn, vchiq_info
and if necessary vchiq_trace.  Also the cat >= test is unnecessary and
this should just use the more common standard logging facilities.

vchiq_loud_error is IMO unnecessary.

Also several of the uses of these macros already have '\n' terminations
so that just adds unnecessary blank lines in the logging.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help