linux-kernel - Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message

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]

Date: Fri, 31 May 2024 09:45:10 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>
Cc: andy@...nel.org, tzimmermann@...e.de, ojeda@...nel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] auxdisplay: linedisp: Support configuring the boot message

Hi Chris,

On Fri, May 31, 2024 at 7:28 AM Chris Packham
<chris.packham@...iedtelesis.co.nz> wrote:
> Like we do for charlcd, allow the configuration of the initial message
> on line-display devices.
>
> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>

Thanks for your patch!

> --- a/drivers/auxdisplay/line-display.c
> +++ b/drivers/auxdisplay/line-display.c
> @@ -8,7 +8,9 @@
>   * Copyright (C) 2021 Glider bv
>   */
>
> +#ifndef CONFIG_PANEL_BOOT_MESSAGE
>  #include <generated/utsrelease.h>
> +#endif

The #ifndef/#endif is not really needed.

>  #include <linux/container_of.h>
>  #include <linux/device.h>
> @@ -312,6 +314,12 @@ static int linedisp_init_map(struct linedisp *linedisp)
>         return 0;
>  }
>
> +#ifdef CONFIG_PANEL_BOOT_MESSAGE
> +#define LINE_DISP_INIT_TEXT CONFIG_PANEL_BOOT_MESSAGE

So the user has to add extra spaces at the end when needed.
This makes sense, as they are not always needed (e.g. when the length of
the message matches the display size, no scrolling is needed/wanted).
I have verified that Kconfig actually preserves such spaces.

Nit: this is the only definition having an underscore between "line"
and "disp".

> +#else
> +#define LINE_DISP_INIT_TEXT "Linux " UTS_RELEASE "       "
> +#endif

I'd rather move this up, next to the other definitions at the top of
the file.

> +
>  /**
>   * linedisp_register - register a character line display
>   * @linedisp: pointer to character line display structure

As I see no real deficiencies:
Reviewed-by: Geert Uytterhoeven <geert@...ux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68korg

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds