linux-kernel - Re: [PATCH 4/7] fbdev/core: Move logo functions into separate source file

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87fs3r4no2.fsf@minerva.mail-host-address-is-not-set>
Date:   Wed, 06 Sep 2023 12:10:53 +0200
From:   Javier Martinez Canillas <javierm@...hat.com>
To:     Thomas Zimmermann <tzimmermann@...e.de>, deller@....de,
        daniel@...ll.ch, sam@...nborg.org, linux-fbdev@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Cc:     linux-kernel@...r.kernel.org,
        Thomas Zimmermann <tzimmermann@...e.de>
Subject: Re: [PATCH 4/7] fbdev/core: Move logo functions into separate
 source file

Thomas Zimmermann <tzimmermann@...e.de> writes:

> Move the fbdev function for displaying boot-up logos into their
> own file fb_logo.c. The file can later be build depending on the
> state of CONFIG_LOGO. No functional changes.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@...e.de>
> ---
>  drivers/video/fbdev/core/Makefile      |   1 +
>  drivers/video/fbdev/core/fb_internal.h |   6 +
>  drivers/video/fbdev/core/fb_logo.c     | 533 +++++++++++++++++++++++++
>  drivers/video/fbdev/core/fbmem.c       | 524 ------------------------
>  include/linux/fb.h                     |   5 -
>  5 files changed, 540 insertions(+), 529 deletions(-)
>  create mode 100644 drivers/video/fbdev/core/fb_logo.c
>
> diff --git a/drivers/video/fbdev/core/Makefile b/drivers/video/fbdev/core/Makefile
> index edfde2948e5c..adce31155e92 100644
> --- a/drivers/video/fbdev/core/Makefile
> +++ b/drivers/video/fbdev/core/Makefile
> @@ -2,6 +2,7 @@
>  obj-$(CONFIG_FB_NOTIFY)           += fb_notify.o
>  obj-$(CONFIG_FB_CORE)             += fb.o
>  fb-y                              := fb_info.o \
> +                                     fb_logo.o \

Maybe instead of adding this unconditionally you could only add it when
CONFIG_LOGO is enabled ?

[...]

> diff --git a/drivers/video/fbdev/core/fb_logo.c b/drivers/video/fbdev/core/fb_logo.c
> new file mode 100644
> index 000000000000..76ba5a2bebae
> --- /dev/null
> +++ b/drivers/video/fbdev/core/fb_logo.c
> @@ -0,0 +1,533 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/export.h>
> +#include <linux/fb.h>
> +#include <linux/linux_logo.h>
> +
> +bool fb_center_logo __read_mostly;
> +int fb_logo_count __read_mostly = -1;
> +
> +#ifdef CONFIG_LOGO

[...]

> +#else
> +int fb_prepare_logo(struct fb_info *info, int rotate)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(fb_prepare_logo);
> +
> +int fb_show_logo(struct fb_info *info, int rotate)
> +{
> +	return 0;
> +}
> +EXPORT_SYMBOL(fb_show_logo);

I would move the CONFIG_LOGO #ifdefery to the header file and make these a
static inline function, instead of having the stubs here and exporting
symbols for functions that are a no-op.

I think that will also fix the issues that the robot complained about.

Other than that, the patch looks good to me.

Acked-by: Javier Martinez Canillas <javierm@...hat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat