linux-kernel - Re: [PATCH] kdb: utilize more readable control characters macro in kdb io

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: <CAD=FV=V6B9mAb-EQaK1K+pa4duzOGmz=dt4QUFVVGUbq7NnK3Q@mail.gmail.com>
Date: Wed, 27 Nov 2024 12:55:10 -0800
From: Doug Anderson <dianders@...omium.org>
To: Nir Lichtman <nir@...htman.org>
Cc: jason.wessel@...driver.com, danielt@...nel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kdb: utilize more readable control characters macro in
 kdb io

Hi,

On Thu, Nov 21, 2024 at 12:45 PM Nir Lichtman <nir@...htman.org> wrote:
>
> @@ -267,7 +267,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
>         if (key != 9)

FWIW, the "9" above is better as CTRL_KEY('I').


> @@ -176,14 +174,16 @@ int kdb_get_kbd_char(void)
>         case KT_LATIN:
>                 switch (keychar) {
>                 /* non-printable supported control characters */
> -               case CTRL('A'): /* Home */
> -               case CTRL('B'): /* Left */
> -               case CTRL('D'): /* Del */
> -               case CTRL('E'): /* End */
> -               case CTRL('F'): /* Right */
> -               case CTRL('I'): /* Tab */
> -               case CTRL('N'): /* Down */
> -               case CTRL('P'): /* Up */
> +               case CTRL_KEY('A'): /* Home */
> +               case CTRL_KEY('B'): /* Left */
> +               case CTRL_KEY('D'): /* Del */
> +               case CTRL_KEY('E'): /* End */
> +               case CTRL_KEY('F'): /* Right */
> +               case CTRL_KEY('I'): /* Tab */
> +               case CTRL_KEY('K'):
> +               case CTRL_KEY('N'): /* Down */
> +               case CTRL_KEY('P'): /* Up */
> +               case CTRL_KEY('U'):

You snuck in a functionality change (adding Ctrl-K and Ctrl-U) here
that should be in a separate patch.

Otherwise this looks nice to me.

-Doug