[PATCH 3/4] drm/encoder: Make init functions panic consitently and explicitly

Maxime Ripard mripard at kernel.org
Wed Dec 6 11:13:50 UTC 2023


All of the current encoder init / allocation functions behave slightly
differently when it comes to argument sanitizing:

 - drm_encoder_init() implicitly panics if the drm_device pointer, or
   the drm_encoder pointer are NULL, and calls WARN_ON if there's no
   destroy implementation but goes on with the initialization.

 - drmm_encoder_init() implicitly panics if the drm_device pointer is
   NULL, and calls WARN_ON and errors out if the drm_encoder_funcs
   pointer is NULL or if there's no destroy implementation.

 - drmm_encoder_alloct() implicitly panics if the drm_device pointer is
   NULL, and calls WARN_ON and errors out if the drm_encoder_funcs
   pointer is NULL or if there's no destroy implementation.

The current consensus is that the drm_device pointer, the
drm_encoder_funcs pointer, and the drm_encoder pointer if relevant,
should be considered pre-requisite and the function should panic if we
encounter such a situation, and that returning an error in such a
situation is not welcome.

Let's make all functions consider those three pointers to be always set
and explicitly panic if they aren't. And let's document that behaviour
too.

Link: https://lore.kernel.org/dri-devel/20231128-kms-hdmi-connector-state-v4-5-c7602158306e@kernel.org/
Signed-off-by: Maxime Ripard <mripard at kernel.org>
---
 drivers/gpu/drm/drm_encoder.c | 19 +++++++++++++++++--
 include/drm/drm_encoder.h     |  6 ++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
index 8f2bc6a28482..bb120b0814a3 100644
--- a/drivers/gpu/drm/drm_encoder.c
+++ b/drivers/gpu/drm/drm_encoder.c
@@ -159,6 +159,9 @@ static int __drm_encoder_init(struct drm_device *dev,
  *
  * Returns:
  * Zero on success, error code on failure.
+ *
+ * Panics:
+ * If @dev, @encoder, or @funcs are NULL.
  */
 int drm_encoder_init(struct drm_device *dev,
 		     struct drm_encoder *encoder,
@@ -168,6 +171,9 @@ int drm_encoder_init(struct drm_device *dev,
 	va_list ap;
 	int ret;
 
+	BUG_ON(!dev);
+	BUG_ON(!encoder);
+	BUG_ON(!funcs);
 	WARN_ON(!funcs->destroy);
 
 	va_start(ap, name);
@@ -227,8 +233,7 @@ static int __drmm_encoder_init(struct drm_device *dev,
 {
 	int ret;
 
-	if (drm_WARN_ON(dev, funcs && funcs->destroy))
-		return -EINVAL;
+	drm_WARN_ON(dev, funcs->destroy);
 
 	ret = __drm_encoder_init(dev, encoder, funcs, encoder_type, name, args);
 	if (ret)
@@ -250,6 +255,9 @@ void *__drmm_encoder_alloc(struct drm_device *dev, size_t size, size_t offset,
 	va_list ap;
 	int ret;
 
+	BUG_ON(!dev);
+	BUG_ON(!funcs);
+
 	container = drmm_kzalloc(dev, size, GFP_KERNEL);
 	if (!container)
 		return ERR_PTR(-ENOMEM);
@@ -283,6 +291,9 @@ EXPORT_SYMBOL(__drmm_encoder_alloc);
  *
  * Returns:
  * Zero on success, error code on failure.
+ *
+ * Panics:
+ * If @dev, @encoder, or @funcs are NULL.
  */
 int drmm_encoder_init(struct drm_device *dev, struct drm_encoder *encoder,
 		      const struct drm_encoder_funcs *funcs,
@@ -291,6 +302,10 @@ int drmm_encoder_init(struct drm_device *dev, struct drm_encoder *encoder,
 	va_list ap;
 	int ret;
 
+	BUG_ON(!dev);
+	BUG_ON(!encoder);
+	BUG_ON(!funcs);
+
 	va_start(ap, name);
 	ret = __drmm_encoder_init(dev, encoder, funcs, encoder_type, name, ap);
 	va_end(ap);
diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h
index 977a9381c8ba..1dbebbc36dd4 100644
--- a/include/drm/drm_encoder.h
+++ b/include/drm/drm_encoder.h
@@ -238,6 +238,9 @@ void *__drmm_encoder_alloc(struct drm_device *dev,
  *
  * Returns:
  * Pointer to new encoder, or ERR_PTR on failure.
+ *
+ * Panics:
+ * If @dev, or @funcs are NULL.
  */
 #define drmm_encoder_alloc(dev, type, member, funcs, encoder_type, name, ...) \
 	((type *)__drmm_encoder_alloc(dev, sizeof(type), \
@@ -256,6 +259,9 @@ void *__drmm_encoder_alloc(struct drm_device *dev,
  *
  * Returns:
  * Pointer to the new drm_encoder struct, or ERR_PTR on failure.
+ *
+ * Panics:
+ * If @dev, or @funcs are NULL.
  */
 #define drmm_plain_encoder_alloc(dev, funcs, encoder_type, name, ...) \
 	((struct drm_encoder *) \
-- 
2.43.0



More information about the dri-devel mailing list