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/[email protected]/ Signed-off-by: Maxime Ripard <[email protected]> --- 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
