All of the current CRTC init / allocation functions behave slightly differently when it comes to argument sanitizing:
- drm_crtc_init_with_planes() implicitly panics if the drm_device pointer, the drm_crtc pointer, or the drm_crtc_funcs pointer are NULL, and calls WARN_ON if there's no destroy implementation but goes on with the initialization. - drmm_crtc_init_with_planes() implicitly panics if the drm_device pointer, the drm_crtc pointer, or the drm_crtc_funcs pointer are NULL, and calls WARN_ON if there's no destroy implementation but goes on with the initialization. - drmm_crtc_alloc_with_planes() implicitly panics if the drm_device pointer, or the drm_crtc pointer are NULL, and calls WARN_ON if the drm_crtc_funcs pointer is NULL or there's no destroy implementation but goes on with the initialization. The current consensus is that the drm_device pointer, the drm_crtc_funcs pointer, and the drm_crtc 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/[email protected]/ Signed-off-by: Maxime Ripard <[email protected]> --- drivers/gpu/drm/drm_crtc.c | 18 ++++++++++++++++-- include/drm/drm_crtc.h | 3 +++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index df9bf3c9206e..87e5877b753d 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -350,6 +350,9 @@ static int __drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc * * * Returns: * Zero on success, error code on failure. + * + * Panics: + * If @dev, @crtc, or @funcs are NULL. */ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, @@ -360,6 +363,9 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, va_list ap; int ret; + BUG_ON(!dev); + BUG_ON(!crtc); + BUG_ON(!funcs); WARN_ON(!funcs->destroy); va_start(ap, name); @@ -432,6 +438,9 @@ static int __drmm_crtc_init_with_planes(struct drm_device *dev, * * Returns: * Zero on success, error code on failure. + * + * Panics: + * If @dev, @crtc, or @funcs are NULL. */ int drmm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, struct drm_plane *primary, @@ -442,6 +451,10 @@ int drmm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, va_list ap; int ret; + BUG_ON(!dev); + BUG_ON(!crtc); + BUG_ON(!funcs); + va_start(ap, name); ret = __drmm_crtc_init_with_planes(dev, crtc, primary, cursor, funcs, name, ap); @@ -465,8 +478,9 @@ void *__drmm_crtc_alloc_with_planes(struct drm_device *dev, va_list ap; int ret; - if (WARN_ON(!funcs || funcs->destroy)) - return ERR_PTR(-EINVAL); + BUG_ON(!dev); + BUG_ON(!funcs); + WARN_ON(funcs->destroy); container = drmm_kzalloc(dev, size, GFP_KERNEL); if (!container) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8b48a1974da3..fdcbc3ac9e08 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1248,6 +1248,9 @@ void *__drmm_crtc_alloc_with_planes(struct drm_device *dev, * * Returns: * Pointer to new crtc, or ERR_PTR on failure. + * + * Panics: + * If @dev or @funcs are NULL. */ #define drmm_crtc_alloc_with_planes(dev, type, member, primary, cursor, funcs, name, ...) \ ((type *)__drmm_crtc_alloc_with_planes(dev, sizeof(type), \ -- 2.43.0
