Re: [PATCH v3 1/9] drm/vc4: Switch to container_of_const

[Javier Martinez Canillas]

Maxime Ripard <maxime@xxxxxxxxxx> writes:

> Hi Javier,
>
> On Sat, Apr 22, 2023 at 07:26:13AM +0200, Javier Martinez Canillas wrote:
>> Maxime Ripard <maxime@xxxxxxxxxx> writes:
>> > container_of_const() allows to preserve the pointer constness and is
>> > thus more flexible than inline functions.
>> >
>> > Let's switch all our instances of container_of() to
>> > container_of_const().
>> >
>> > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
>> > ---
>> 
>> [...]
>> 
>> > -static inline struct vc4_dpi *
>> > -to_vc4_dpi(struct drm_encoder *encoder)
>> > -{
>> > -	return container_of(encoder, struct vc4_dpi, encoder.base);
>> > -}
>> > +#define to_vc4_dpi(_encoder)						\
>> > +	container_of_const(_encoder, struct vc4_dpi, encoder.base)
>> >
>> 
>> A disadvantage of this approach though is that the type checking is lost.
>
> Not entirely, the argument is still type-checked, but yeah, it's true
> for the returned value.
>
>> Since you already had these, I would probably had changed them to return
>> a const pointer and just replace container_of() for container_of_const().
>> 
>> But I see that there are a lot of patches from Greg all over the kernel
>> that do exactly this, dropping static inline functions in favor of using
>> container_of_const() directly. So it seems the convention is what you do.
>
> More importantly, container_of_const() isn't always returning a const
> pointer or always taking a const argument, it's returning the pointer
> with the same const-ness than the argument.
>
> This is why it makes sense to remove the inline function entirely,
> because it removes the main benefit it brings.
>

Got it. Thanks for the explanations.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat