Re: [PATCH v4 02/12] drm/client: Restrict the plane_state scope

[Jani Nikula <jani.nikula@linux.intel.com>]

On Fri, 14 Jun 2019, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> The drm_client_modeset_commit_atomic function uses two times the
> plane_state variable in inner blocks of code, but the variable has a scope
> global to this function.
>
> This will lead to inadvertent devs to reuse the variable in the second
> block with the value left by the first, without any warning from the
> compiler since value would have been initialized.
>
> Fix this by moving the variable declaration to the proper scope.

This is an improvement, but I'd consider renaming also to not shadow
variables.

BR,
Jani.

>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/gpu/drm/drm_client_modeset.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index 006bf7390e7d..8264c3a732b0 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -861,7 +861,6 @@ EXPORT_SYMBOL(drm_client_panel_rotation);
>  static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool active)
>  {
>  	struct drm_device *dev = client->dev;
> -	struct drm_plane_state *plane_state;
>  	struct drm_plane *plane;
>  	struct drm_atomic_state *state;
>  	struct drm_modeset_acquire_ctx ctx;
> @@ -879,6 +878,8 @@ static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool 
>  	state->acquire_ctx = &ctx;
>  retry:
>  	drm_for_each_plane(plane, dev) {
> +		struct drm_plane_state *plane_state;
> +
>  		plane_state = drm_atomic_get_plane_state(state, plane);
>  		if (IS_ERR(plane_state)) {
>  			ret = PTR_ERR(plane_state);
> @@ -901,6 +902,8 @@ static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool 
>  		unsigned int rotation;
>  
>  		if (drm_client_panel_rotation(mode_set, &rotation)) {
> +			struct drm_plane_state *plane_state;
> +
>  			/* Cannot fail as we've already gotten the plane state above */
>  			plane_state = drm_atomic_get_new_plane_state(state, primary);
>  			plane_state->rotation = rotation;

-- 
Jani Nikula, Intel Open Source Graphics Center

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel