All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <mripard@kernel.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>,
	David Airlie <airlied@linux.ie>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Maxime Ripard <mripard@kernel.org>
Cc: dri-devel@lists.freedesktop.org
Subject: [PATCH 4/4] drm/connector: Make init functions panic consitently and explicitly
Date: Wed,  6 Dec 2023 12:13:51 +0100	[thread overview]
Message-ID: <20231206111351.300225-4-mripard@kernel.org> (raw)
In-Reply-To: <20231206111351.300225-1-mripard@kernel.org>

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

 - drm_connector_init() and drm_connector_init_with_ddc() implicitly
   panic if the drm_device pointer, or the drm_connector pointer are
   NULL, and will call WARN_ON and error out if the
   drm_connector_funcs pointer is NULL or if there's no destroy
   implementation.

 - drmm_connector_init() implicitly panics if the drm_device pointer,
   the drm_connector_funcs pointer, or the drm_connector pointer are
   NULL, and will call WARN_ON and error out if there's a destroy
   implementation.

The current consensus is that the drm_device pointer, the drm_connector
pointer, and the drm_connector_funcs pointer 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@kernel.org>
---
 drivers/gpu/drm/drm_connector.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b0516505f7ae..e7a463db11ea 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -351,14 +351,19 @@ static int __drm_connector_init(struct drm_device *dev,
  *
  * Returns:
  * Zero on success, error code on failure.
+ *
+ * Panics:
+ * If @dev, @connector, or @funcs are NULL.
  */
 int drm_connector_init(struct drm_device *dev,
 		       struct drm_connector *connector,
 		       const struct drm_connector_funcs *funcs,
 		       int connector_type)
 {
-	if (drm_WARN_ON(dev, !(funcs && funcs->destroy)))
-		return -EINVAL;
+	BUG_ON(!dev);
+	BUG_ON(!connector);
+	BUG_ON(!funcs);
+	drm_WARN_ON(dev, !funcs->destroy);
 
 	return __drm_connector_init(dev, connector, funcs, connector_type, NULL);
 }
@@ -387,6 +392,9 @@ EXPORT_SYMBOL(drm_connector_init);
  *
  * Returns:
  * Zero on success, error code on failure.
+ *
+ * Panics:
+ * If @dev, @connector, or @funcs are NULL.
  */
 int drm_connector_init_with_ddc(struct drm_device *dev,
 				struct drm_connector *connector,
@@ -394,8 +402,10 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
 				int connector_type,
 				struct i2c_adapter *ddc)
 {
-	if (drm_WARN_ON(dev, !(funcs && funcs->destroy)))
-		return -EINVAL;
+	BUG_ON(!dev);
+	BUG_ON(!connector);
+	BUG_ON(!funcs);
+	drm_WARN_ON(dev, !funcs->destroy);
 
 	return __drm_connector_init(dev, connector, funcs, connector_type, ddc);
 }
@@ -427,6 +437,9 @@ static void drm_connector_cleanup_action(struct drm_device *dev,
  *
  * Returns:
  * Zero on success, error code on failure.
+ *
+ * Panics:
+ * If @dev, @connector, or @funcs are NULL.
  */
 int drmm_connector_init(struct drm_device *dev,
 			struct drm_connector *connector,
@@ -436,8 +449,10 @@ int drmm_connector_init(struct drm_device *dev,
 {
 	int ret;
 
-	if (drm_WARN_ON(dev, funcs && funcs->destroy))
-		return -EINVAL;
+	BUG_ON(!dev);
+	BUG_ON(!connector);
+	BUG_ON(!funcs);
+	drm_WARN_ON(dev, funcs->destroy);
 
 	ret = __drm_connector_init(dev, connector, funcs, connector_type, ddc);
 	if (ret)
-- 
2.43.0


  parent reply	other threads:[~2023-12-06 11:14 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 11:13 [PATCH 1/4] drm/plane: Make init functions panic consitently and explicitly Maxime Ripard
2023-12-06 11:13 ` [PATCH 2/4] drm/crtc: " Maxime Ripard
2023-12-06 11:13 ` [PATCH 3/4] drm/encoder: " Maxime Ripard
2023-12-06 11:13 ` Maxime Ripard [this message]
2023-12-07 10:18 ` [PATCH 1/4] drm/plane: " Jani Nikula
2023-12-07 13:05   ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231206111351.300225-4-mripard@kernel.org \
    --to=mripard@kernel.org \
    --cc=airlied@linux.ie \
    --cc=daniel.vetter@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.