1. Patchew
  2. linux
  3. View series

[PATCH] kdb: utilize more readable control characters macro in kdb io

Nir Lichtman posted 1 patch 1 year, 2 months ago
There is a newer version of this series
kernel/debug/kdb/kdb_io.c       | 36 ++++++++++++++++-----------------
kernel/debug/kdb/kdb_keyboard.c | 36 ++++++++++++++++-----------------
kernel/debug/kdb/kdb_private.h  |  2 ++
3 files changed, 38 insertions(+), 36 deletions(-)
[PATCH] kdb: utilize more readable control characters macro in kdb io
Posted by Nir Lichtman 1 year, 2 months ago 
Continuing the previous refactor in kdb_keyboard.c of adding the CTRL
macro, this patch moves the macro to a common header and utilizes this
pattern in kdb_io.c as well, resulting in more readable code.

I have added the _KEY suffix to make the macro more clear, considering
it is now common.

Signed-off-by: Nir Lichtman <nir@lichtman.org>
---
 kernel/debug/kdb/kdb_io.c       | 36 ++++++++++++++++-----------------
 kernel/debug/kdb/kdb_keyboard.c | 36 ++++++++++++++++-----------------
 kernel/debug/kdb/kdb_private.h  |  2 ++
 3 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index 6a77f1c779c4..3a459415b2fc 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -76,13 +76,13 @@ static int kdb_handle_escape(char *buf, size_t sz)
 	case 3:
 		switch (*lastkey) {
 		case 'A': /* \e[A, up arrow */
-			return 16;
+			return CTRL_KEY('P');
 		case 'B': /* \e[B, down arrow */
-			return 14;
+			return CTRL_KEY('N');
 		case 'C': /* \e[C, right arrow */
-			return 6;
+			return CTRL_KEY('F');
 		case 'D': /* \e[D, left arrow */
-			return 2;
+			return CTRL_KEY('B');
 		case '1': /* \e[<1,3,4>], may be home, del, end */
 		case '3':
 		case '4':
@@ -94,11 +94,11 @@ static int kdb_handle_escape(char *buf, size_t sz)
 		if (*lastkey == '~') {
 			switch (buf[2]) {
 			case '1': /* \e[1~, home */
-				return 1;
+				return CTRL_KEY('A');
 			case '3': /* \e[3~, del */
-				return 4;
+				return CTRL_KEY('D');
 			case '4': /* \e[4~, end */
-				return 5;
+				return CTRL_KEY('E');
 			}
 		}
 		break;
@@ -267,7 +267,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
 	if (key != 9)
 		tab = 0;
 	switch (key) {
-	case 8: /* backspace */
+	case CTRL_KEY('H'): /* backspace */
 		if (cp > buffer) {
 			memmove(cp-1, cp, lastchar - cp + 1);
 			lastchar--;
@@ -276,8 +276,8 @@ static char *kdb_read(char *buffer, size_t bufsize)
 			kdb_position_cursor(kdb_prompt_str, buffer, cp);
 		}
 		break;
-	case 10: /* linefeed */
-	case 13: /* carriage return */
+	case CTRL_KEY('J'): /* linefeed */
+	case CTRL_KEY('M'): /* carriage return */
 		*lastchar++ = '\n';
 		*lastchar++ = '\0';
 		if (!KDB_STATE(KGDB_TRANS)) {
@@ -286,7 +286,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
 		}
 		kdb_printf("\n");
 		return buffer;
-	case 4: /* Del */
+	case CTRL_KEY('D'): /* Del */
 		if (cp < lastchar) {
 			memmove(cp, cp+1, lastchar - cp);
 			lastchar--;
@@ -294,39 +294,39 @@ static char *kdb_read(char *buffer, size_t bufsize)
 			kdb_position_cursor(kdb_prompt_str, buffer, cp);
 		}
 		break;
-	case 1: /* Home */
+	case CTRL_KEY('A'): /* Home */
 		if (cp > buffer) {
 			cp = buffer;
 			kdb_position_cursor(kdb_prompt_str, buffer, cp);
 		}
 		break;
-	case 5: /* End */
+	case CTRL_KEY('E'): /* End */
 		if (cp < lastchar) {
 			kdb_printf("%s", cp);
 			cp = lastchar;
 		}
 		break;
-	case 2: /* Left */
+	case CTRL_KEY('B'): /* Left */
 		if (cp > buffer) {
 			kdb_printf("\b");
 			--cp;
 		}
 		break;
-	case 14: /* Down */
-	case 16: /* Up */
+	case CTRL_KEY('N'): /* Down */
+	case CTRL_KEY('P'): /* Up */
 		kdb_printf("\r%*c\r",
 			   (int)(strlen(kdb_prompt_str) + (lastchar - buffer)),
 			   ' ');
 		*lastchar = (char)key;
 		*(lastchar+1) = '\0';
 		return lastchar;
-	case 6: /* Right */
+	case CTRL_KEY('F'): /* Right */
 		if (cp < lastchar) {
 			kdb_printf("%c", *cp);
 			++cp;
 		}
 		break;
-	case 9: /* Tab */
+	case CTRL_KEY('I'): /* Tab */
 		if (tab < 2)
 			++tab;
 
diff --git a/kernel/debug/kdb/kdb_keyboard.c b/kernel/debug/kdb/kdb_keyboard.c
index 3a74604fdb8a..b3d69f6ed30a 100644
--- a/kernel/debug/kdb/kdb_keyboard.c
+++ b/kernel/debug/kdb/kdb_keyboard.c
@@ -25,8 +25,6 @@
 #define KBD_STAT_OBF 		0x01	/* Keyboard output buffer full */
 #define KBD_STAT_MOUSE_OBF	0x20	/* Mouse output buffer full */
 
-#define CTRL(c) ((c) - 64)
-
 static int kbd_exists;
 static int kbd_last_ret;
 
@@ -128,21 +126,21 @@ int kdb_get_kbd_char(void)
 	/* Translate special keys to equivalent CTRL control characters */
 	switch (scancode) {
 	case 0xF: /* Tab */
-		return CTRL('I');
+		return CTRL_KEY('I');
 	case 0x53: /* Del */
-		return CTRL('D');
+		return CTRL_KEY('D');
 	case 0x47: /* Home */
-		return CTRL('A');
+		return CTRL_KEY('A');
 	case 0x4F: /* End */
-		return CTRL('E');
+		return CTRL_KEY('E');
 	case 0x4B: /* Left */
-		return CTRL('B');
+		return CTRL_KEY('B');
 	case 0x48: /* Up */
-		return CTRL('P');
+		return CTRL_KEY('P');
 	case 0x50: /* Down */
-		return CTRL('N');
+		return CTRL_KEY('N');
 	case 0x4D: /* Right */
-		return CTRL('F');
+		return CTRL_KEY('F');
 	}
 
 	if (scancode == 0xe0)
@@ -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'):
 			return keychar;
 		}
 
diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h
index d2520d72b1f5..1f29df5d89b8 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -227,6 +227,8 @@ extern char kdb_prompt_str[];
 
 #define	KDB_WORD_SIZE	((int)sizeof(unsigned long))
 
+#define CTRL_KEY(c) ((c) - 64)
+
 #endif /* CONFIG_KGDB_KDB */
 
 #define kdb_func_printf(format, args...) \
-- 
2.39.2
Re: [PATCH] kdb: utilize more readable control characters macro in kdb io
Posted by Doug Anderson 1 year, 2 months ago 
Hi,

On Thu, Nov 21, 2024 at 12:45 PM Nir Lichtman <nir@lichtman.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
Re: [PATCH] kdb: utilize more readable control characters macro in kdb io
Posted by Nir Lichtman 1 year, 2 months ago 
On Wed, Nov 27, 2024 at 12:55:10PM -0800, Doug Anderson wrote:
> Hi,
> 
> On Thu, Nov 21, 2024 at 12:45 PM Nir Lichtman <nir@lichtman.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').

Right, missed that will fix.

> 
> 
> > @@ -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.
> 

Oops, those are left overs of something else I was trying to do, will remove.

> Otherwise this looks nice to me.

Thanks! Will send out a v2 with fixes shortly,
Nir

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.