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

[Thomas Zimmermann]

Hi

thanks for the fix.

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

with a remark below.

Am 09.04.25 um 21:03 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.

Right, the variable mem_flags should always be set and thus screen_info 
should be mapped. I never got this error with gcc.

>
> Initialize screen_base to NULL and add a defensive print in case
> mem_flags were ever returned without one of the four valid values.
>
> Fixes: 32ae90c66fb6 ("drm/sysfb: Add efidrm for EFI displays")
> Signed-off-by: Nathan Chancellor <nathan at 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..7d820f42956b 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_warn(dev, "unhandled mem_flags: 0x%llx\n", mem_flags);


This message should be  something like drm_err("invalid mem_flags ..."). 
It's really a driver bug.

Would you send an update?

Best regards
Thomas

>   	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)