kdb: Remove special case logic from kdb_read()

kdb_read() contains special case logic to force it exit after reading a single character. We can remove all the special case logic by directly calling the function to read a single character instead. This also allows us to tidy up the function prototype which, because it now matches getchar(), we can also rename in order to make its role clearer. This does involve some extra code to handle btaprompt properly but we don't mind the new lines of code here because the old code had some interesting problems (bad newline handling, treating unexpected characters like <cr>).

Daniel Thompson committed 7 years ago

Commit 4f27e821 parent d04213a3 files · +42 −42Browse files

Showing the changes to kernel/debug (3 of 3 files). Show all

Unified · Split

3 files

kernel/debug/kdb/kdb_bt.c +14 −8
kernel/debug/kdb/kdb_io.c +27 −34
kernel/debug/kdb/kdb_private.h +1 −0

kernel/debug/kdb/kdb_bt.c+14−8

@@ -75,9 +75,10 @@ static void kdb_show_stack(struct task_struct p, void addr) · 74 unmodified lines
75	75	static int
76	76	kdb_bt1(struct task_struct *p, unsigned long mask, bool btaprompt)
77	77	{
78		− char buffer[2];
79		− if (kdb_getarea(buffer[0], (unsigned long)p) \|\|
80		− kdb_getarea(buffer[0], (unsigned long)(p+1)-1))
	78	+ char ch;
	79	+
	80	+ if (kdb_getarea(ch, (unsigned long)p) \|\|
	81	+ kdb_getarea(ch, (unsigned long)(p+1)-1))
81	82	return KDB_BADADDR;
82	83	if (!kdb_task_state(p, mask))
83	84	return 0;
@@ -85,12 +86,17 @@ kdb_bt1(struct task_struct *p, unsigned long mask, bool btaprompt) · 1 unmodified line
85	86	kdb_ps1(p);
86	87	kdb_show_stack(p, NULL);
87	88	if (btaprompt) {
88		− kdb_getstr(buffer, sizeof(buffer),
89		− "Enter <q> to end, <cr> to continue:");
90		− if (buffer[0] == 'q') {
91		− kdb_printf("\n");
	89	+ kdb_printf("Enter <q> to end, <cr> or <space> to continue:");
	90	+ do {
	91	+ ch = kdb_getchar();
	92	+ } while (!strchr("\r\n q", ch));
	93	+ kdb_printf("\n");
	94	+
	95	+ /* reset the pager */
	96	+ kdb_nextline = 1;
	97	+
	98	+ if (ch == 'q')
92	99	return 1;
93		− }
94	100	}
95	101	touch_nmi_watchdog();
96	102	return 0;

kernel/debug/kdb/kdb_io.c+27−34

@@ -108,7 +108,22 @@ static int kdb_handle_escape(char *buf, size_t sz) · 107 unmodified lines
108	108	return -1;
109	109	}
110	110
111		−static int kdb_read_get_key(char *buffer, size_t bufsize)
	111	+/**
	112	+ * kdb_getchar() - Read a single character from a kdb console (or consoles).
	113	+ *
	114	+ * Other than polling the various consoles that are currently enabled,
	115	+ * most of the work done in this function is dealing with escape sequences.
	116	+ *
	117	+ * An escape key could be the start of a vt100 control sequence such as \e[D
	118	+ * (left arrow) or it could be a character in its own right. The standard
	119	+ * method for detecting the difference is to wait for 2 seconds to see if there
	120	+ * are any other characters. kdb is complicated by the lack of a timer service
	121	+ * (interrupts are off), by multiple input sources. Escape sequence processing
	122	+ * has to be done as states in the polling loop.
	123	+ *
	124	+ * Return: The key pressed or a control code derived from an escape sequence.
	125	+ */
	126	+char kdb_getchar(void)
112	127	{
113	128	#define ESCAPE_UDELAY 1000
114	129	#define ESCAPE_DELAY (21000000/ESCAPE_UDELAY) / 2 seconds worth of udelays */
@@ -126,7 +141,6 @@ static int kdb_read_get_key(char *buffer, size_t bufsize) · 11 unmodified lines
126	141	}
127	142
128	143	key = (*f)();
129		−
130	144	if (key == -1) {
131	145	if (escape_delay) {
132	146	udelay(ESCAPE_UDELAY);
@@ -136,14 +150,6 @@ static int kdb_read_get_key(char *buffer, size_t bufsize) · 3 unmodified lines
136	150	continue;
137	151	}
138	152
139		− if (bufsize <= 2) {
140		− if (key == '\r')
141		− key = '\n';
142		− *buffer++ = key;
143		− *buffer = '\0';
144		− return -1;
145		− }
146		−
147	153	if (escape_delay == 0 && key == '\e') {
148	154	escape_delay = ESCAPE_DELAY;
149	155	ped = escape_data;
@@ -184,17 +190,7 @@ static int kdb_read_get_key(char *buffer, size_t bufsize) · 34 unmodified lines
184	190	* function. It is not reentrant - it relies on the fact
185	191	* that while kdb is running on only one "master debug" cpu.
186	192	* Remarks:
187		− *
188		− * The buffer size must be >= 2. A buffer size of 2 means that the caller only
189		− * wants a single key.
190		− *
191		− * An escape key could be the start of a vt100 control sequence such as \e[D
192		− * (left arrow) or it could be a character in its own right. The standard
193		− * method for detecting the difference is to wait for 2 seconds to see if there
194		− * are any other characters. kdb is complicated by the lack of a timer service
195		− * (interrupts are off), by multiple input sources and by the need to sometimes
196		− * return after just one key. Escape sequence processing has to be done as
197		− * states in the polling loop.
	193	+ * The buffer size must be >= 2.
198	194	*/
199	195
200	196	static char kdb_read(char buffer, size_t bufsize)
@@ -229,9 +225,7 @@ static char kdb_read(char buffer, size_t bufsize) · 28 unmodified lines
229	225	*cp = '\0';
230	226	kdb_printf("%s", buffer);
231	227	poll_again:
232		− key = kdb_read_get_key(buffer, bufsize);
233		− if (key == -1)
234		− return buffer;
	228	+ key = kdb_getchar();
235	229	if (key != 9)
236	230	tab = 0;
237	231	switch (key) {
@@ -742,7 +736,7 @@ kdb_printit: · 504 unmodified lines
742	736
743	737	/* check for having reached the LINES number of printed lines */
744	738	if (kdb_nextline >= linecount) {
745		− char buf1[16] = "";
	739	+ char ch;
746	740
747	741	/* Watch out for recursion here. Any routine that calls
748	742	* kdb_printf will come back through here. And kdb_read
@@ -777,39 +771,38 @@ kdb_printit: · 28 unmodified lines
777	771	if (logging)
778	772	printk("%s", moreprompt);
779	773
780		− kdb_read(buf1, 2); /* '2' indicates to return
781		− * immediately after getting one key. */
	774	+ ch = kdb_getchar();
782	775	kdb_nextline = 1; /* Really set output line 1 */
783	776
784	777	/* empty and reset the buffer: */
785	778	kdb_buffer[0] = '\0';
786	779	next_avail = kdb_buffer;
787	780	size_avail = sizeof(kdb_buffer);
788		− if ((buf1[0] == 'q') \|\| (buf1[0] == 'Q')) {
	781	+ if ((ch == 'q') \|\| (ch == 'Q')) {
789	782	/* user hit q or Q */
790	783	KDB_FLAG_SET(CMD_INTERRUPT); /* command interrupted */
791	784	KDB_STATE_CLEAR(PAGER);
792	785	/* end of command output; back to normal mode */
793	786	kdb_grepping_flag = 0;
794	787	kdb_printf("\n");
795		− } else if (buf1[0] == ' ') {
	788	+ } else if (ch == ' ') {
796	789	kdb_printf("\r");
797	790	suspend_grep = 1; /* for this recursion */
798		− } else if (buf1[0] == '\n') {
	791	+ } else if (ch == '\n' \|\| ch == '\r') {
799	792	kdb_nextline = linecount - 1;
800	793	kdb_printf("\r");
801	794	suspend_grep = 1; /* for this recursion */
802		− } else if (buf1[0] == '/' && !kdb_grepping_flag) {
	795	+ } else if (ch == '/' && !kdb_grepping_flag) {
803	796	kdb_printf("\r");
804	797	kdb_getstr(kdb_grep_string, KDB_GREP_STRLEN,
805	798	kdbgetenv("SEARCHPROMPT") ?: "search> ");
806	799	*strchrnul(kdb_grep_string, '\n') = '\0';
807	800	kdb_grepping_flag += KDB_GREPPING_FLAG_SEARCH;
808	801	suspend_grep = 1; /* for this recursion */
809		− } else if (buf1[0] && buf1[0] != '\n') {
810		− /* user hit something other than enter */
	802	+ } else if (ch) {
	803	+ /* user hit something unexpected */
811	804	suspend_grep = 1; /* for this recursion */
812		− if (buf1[0] != '/')
	805	+ if (ch != '/')
813	806	kdb_printf(
814	807	"\nOnly 'q', 'Q' or '/' are processed at "
815	808	"more prompt, input ignored\n");

kernel/debug/kdb/kdb_private.h+1−0

@@ -210,6 +210,7 @@ extern void kdb_ps1(const struct task_struct *p); · 209 unmodified lines
210	210	extern void kdb_print_nameval(const char *name, unsigned long val);
211	211	extern void kdb_send_sig(struct task_struct *p, int sig);
212	212	extern void kdb_meminfo_proc_show(void);
	213	+extern char kdb_getchar(void);
213	214	extern char kdb_getstr(char , size_t, const char *);
214	215	extern void kdb_gdb_state_pass(char *buf);
215	216