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

Joe Perches joe at perches.com
Sun Oct 24 16:56:55 PDT 2021


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.





More information about the linux-arm-kernel mailing list