lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <a7d1136e22bdb37170a0fe9a9443ba0cdf5dcd32.camel@perches.com> Date: Sun, 24 Oct 2021 16:56:55 -0700 From: Joe Perches <joe@...ches.com> To: Gaston Gonzalez <gascoar@...il.com>, linux-staging@...ts.linux.dev Cc: gregkh@...uxfoundation.org, nsaenz@...nel.org, stefan.wahren@...e.com, arnd@...db.de, dan.carpenter@...cle.com, ojaswin98@...il.com, amarjargal16@...il.com, linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org, bcm-kernel-feedback-list@...adcom.com, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2 8/8] staging: vchiq_core: fix quoted strings split across lines 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.
Powered by blists - more mailing lists