| 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: <20250424093839.27451-1-rf@opensource.cirrus.com>
Date: Thu, 24 Apr 2025 10:38:39 +0100
From: Richard Fitzgerald <rf@...nsource.cirrus.com>
To: gregkh@...uxfoundation.org, rafael@...nel.org, dakr@...nel.org
Cc: linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com,
Richard Fitzgerald <rf@...nsource.cirrus.com>
Subject: [PATCH] driver core: Introduce dev_err_ret() and dev_warn_ret()
Add two helper functions dev_err_ret() and dev_warn_ret(). These are
like dev_err_probe() and dev_warn_probe() but there is no special
handling of EPROBE_DEFER.
Although dev_{err,warn}_probe() could be used outside of probe(), it
has the disadvantage that if the error code is unexpectedly EPROBE_DEFER
it would suppress it and also update the deferred probe reason. If code
receives a EPROBE_DEFER in a situation where it cannot handle it, that
really should be logged. There is also the potential for confusion when
seeing a dev_{err,warn}_probe() inside a function that is not actually
part of probe.
They have the same advantage of a standard error message format, and
returning the error value so that code like:
if (err) {
dev_err(dev, ..., err);
return err;
}
can be replaced with
if (err)
return dev_err_ret(dev, err, ...);
The simple cases of dev_err_ret() and dev_warn_ret() could have been
implemented as a trivial inline function to call dev_{err|warn}() and
then return the passed error value. But it is useful to keep the same
message format as dev_{err|warn}_probe() and re-using that code avoids
scattering duplicate format strings around. Also it should slightly
reduce the size of the error path code in the kernel because the compiler
does not have to preserve the error code value for the final return.
Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
---
drivers/base/core.c | 112 +++++++++++++++++++++++++++++++++----
include/linux/dev_printk.h | 3 +
2 files changed, 105 insertions(+), 10 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index d2f9d3a59d6b..a8bd8d02e0eb 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4961,8 +4961,12 @@ define_dev_printk_level(_dev_info, KERN_INFO);
#endif
-static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
- const char *fmt, va_list vargsp)
+#define DEV_LOG_ERRVAL_F_PROBE 0x1
+#define DEV_LOG_ERRVAL_F_FATAL 0x2
+
+static void __dev_log_msg_with_errval(const struct device *dev, int err,
+ unsigned int flags, const char *fmt,
+ va_list vargsp)
{
struct va_format vaf;
va_list vargs;
@@ -4983,18 +4987,20 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
vaf.va = &vargs;
switch (err) {
- case -EPROBE_DEFER:
- device_set_deferred_probe_reason(dev, &vaf);
- dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
- break;
-
case -ENOMEM:
/* Don't print anything on -ENOMEM, there's already enough output */
break;
+ case -EPROBE_DEFER:
+ if (flags & DEV_LOG_ERRVAL_F_PROBE) {
+ device_set_deferred_probe_reason(dev, &vaf);
+ dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
+ break;
+ }
+ fallthrough;
default:
/* Log fatal final failures as errors, otherwise produce warnings */
- if (fatal)
+ if (flags & DEV_LOG_ERRVAL_F_FATAL)
dev_err(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
else
dev_warn(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
@@ -5044,7 +5050,9 @@ int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
va_start(vargs, fmt);
/* Use dev_err() for logging when err doesn't equal -EPROBE_DEFER */
- __dev_probe_failed(dev, err, true, fmt, vargs);
+ __dev_log_msg_with_errval(dev, err,
+ DEV_LOG_ERRVAL_F_PROBE | DEV_LOG_ERRVAL_F_FATAL,
+ fmt, vargs);
va_end(vargs);
@@ -5092,7 +5100,7 @@ int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
va_start(vargs, fmt);
/* Use dev_warn() for logging when err doesn't equal -EPROBE_DEFER */
- __dev_probe_failed(dev, err, false, fmt, vargs);
+ __dev_log_msg_with_errval(dev, err, DEV_LOG_ERRVAL_F_PROBE, fmt, vargs);
va_end(vargs);
@@ -5100,6 +5108,90 @@ int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
}
EXPORT_SYMBOL_GPL(dev_warn_probe);
+/**
+ * dev_err_ret - log error message with error value and return error value
+ * @dev: the pointer to the struct device
+ * @err: error value to log
+ * @fmt: printf-style format string
+ * @...: arguments as specified in the format string
+ *
+ * This helper implements common pattern for error checking:
+ * print error message with error code and propagate error upwards.
+ * It replaces the following code sequence::
+ *
+ * if (err) {
+ * dev_err(dev, ..., err);
+ * return err;
+ * }
+ *
+ * with::
+ *
+ * if (err)
+ * return dev_err_ret(dev, err, ...);
+ *
+ * The benefit compared to a normal dev_err() is the standardized format
+ * of the error code, which is emitted symbolically (i.e. you get "EAGAIN"
+ * instead of "-35"), and having the error code returned allows more
+ * compact error paths.
+ *
+ * Returns @err.
+ */
+int dev_err_ret(const struct device *dev, int err, const char *fmt, ...)
+{
+ va_list vargs;
+
+ va_start(vargs, fmt);
+
+ __dev_log_msg_with_errval(dev, err, DEV_LOG_ERRVAL_F_FATAL, fmt, vargs);
+
+ va_end(vargs);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(dev_err_ret);
+
+/**
+ * dev_warn_ret - log warning message with error value and return error value
+ * @dev: the pointer to the struct device
+ * @err: error value to log
+ * @fmt: printf-style format string
+ * @...: arguments as specified in the format string
+ *
+ * This helper implements common pattern for error checking:
+ * print warning message with error code and propagate error upwards.
+ * It replaces the following code sequence::
+ *
+ * if (err) {
+ * dev_warn(dev, ..., err);
+ * return err;
+ * }
+ *
+ * with::
+ *
+ * if (err)
+ * return dev_warn_ret(dev, err, ...);
+ *
+ * The benefit compared to a normal dev_warn() is the standardized format
+ * of the error code, which is emitted symbolically (i.e. you get "EAGAIN"
+ * instead of "-35"), and having the error code returned allows more
+ * compact error paths.
+ *
+ * Returns @err.
+ */
+int dev_warn_ret(const struct device *dev, int err, const char *fmt, ...)
+{
+ va_list vargs;
+
+ va_start(vargs, fmt);
+
+ __dev_log_msg_with_errval(dev, err, 0, fmt, vargs);
+
+ va_end(vargs);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(dev_warn_ret);
+
static inline bool fwnode_is_primary(struct fwnode_handle *fwnode)
{
return fwnode && !IS_ERR(fwnode->secondary);
diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
index eb2094e43050..336b2f948e3a 100644
--- a/include/linux/dev_printk.h
+++ b/include/linux/dev_printk.h
@@ -286,4 +286,7 @@ __printf(3, 4) int dev_warn_probe(const struct device *dev, int err, const char
#define dev_err_cast_probe(dev, ___err_ptr, fmt, ...) \
ERR_PTR(dev_err_probe(dev, PTR_ERR(___err_ptr), fmt, ##__VA_ARGS__))
+__printf(3, 4) int dev_err_ret(const struct device *dev, int err, const char *fmt, ...);
+__printf(3, 4) int dev_warn_ret(const struct device *dev, int err, const char *fmt, ...);
+
#endif /* _DEVICE_PRINTK_H_ */
--
2.43.0
Powered by blists - more mailing lists