Re: [PATCH v2] drm/sysfb: efidrm: Avoid clang -Wsometimes-uninitialized in efidrm_device_create()

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi,

thanks for the updated patch.

Am 10.04.25 um 19:43 schrieb Nathan Chancellor:
> Clang warns (or errors with CONFIG_WERROR=y):
>
>    drivers/gpu/drm/sysfb/efidrm.c:353:11: error: variable 'screen_base' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
>      353 |         else if (mem_flags & EFI_MEMORY_WB)
>          |                  ^~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/gpu/drm/sysfb/efidrm.c:356:7: note: uninitialized use occurs here
>      356 |         if (!screen_base)
>          |              ^~~~~~~~~~~
>    drivers/gpu/drm/sysfb/efidrm.c:353:7: note: remove the 'if' if its condition is always true
>      353 |         else if (mem_flags & EFI_MEMORY_WB)
>          |              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      354 |                 screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
>    drivers/gpu/drm/sysfb/efidrm.c:261:27: note: initialize the variable 'screen_base' to silence this warning
>      261 |         void __iomem *screen_base;
>          |                                  ^
>          |                                   = NULL
>
> efidrm_get_mem_flags() can only return a mask that has at least one of
> the tested values set so the else case is impossible but clang's static
> analysis runs before inlining so it cannot know that.
>
> Initialize screen_base to NULL and add a defensive error message in case
> mem_flags were ever returned without one of the four valid values.
>
> Fixes: 32ae90c66fb6 ("drm/sysfb: Add efidrm for EFI displays")
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Best regards
Thomas

> ---
> Changes in v2:
> - Upgrade drm_warn() to drm_err() and adjust message (Thomas).
> - Carry forward Thomas's reviewed-by.
> - Link to v1: https://lore.kernel.org/r/20250409-efidrm-avoid-uninit-screen_info-warning-v1-1-67babb19d831@kernel.org
> ---
>   drivers/gpu/drm/sysfb/efidrm.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
> index af90064a4c04..3cfd5d2cbf48 100644
> --- a/drivers/gpu/drm/sysfb/efidrm.c
> +++ b/drivers/gpu/drm/sysfb/efidrm.c
> @@ -258,7 +258,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>   	struct drm_sysfb_device *sysfb;
>   	struct drm_device *dev;
>   	struct resource *mem = NULL;
> -	void __iomem *screen_base;
> +	void __iomem *screen_base = NULL;
>   	struct drm_plane *primary_plane;
>   	struct drm_crtc *crtc;
>   	struct drm_encoder *encoder;
> @@ -353,6 +353,8 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>   	else if (mem_flags & EFI_MEMORY_WB)
>   		screen_base = devm_memremap(&pdev->dev, mem->start, resource_size(mem),
>   					    MEMREMAP_WB);
> +	else
> +		drm_err(dev, "invalid mem_flags: 0x%llx\n", mem_flags);
>   	if (!screen_base)
>   		return ERR_PTR(-ENOMEM);
>   	iosys_map_set_vaddr_iomem(&sysfb->fb_addr, screen_base);
>
> ---
> base-commit: e8bf4a1bdaeadb28d13b9a2bcfd5910fda06eede
> change-id: 20250409-efidrm-avoid-uninit-screen_info-warning-d62ef67ce3c5
>
> Best regards,

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)