-----Original Message-----
From: Cao, Minggui <minggui.cao@...>
Sent: Saturday, July 23, 2022 3:27 AM
To: Dong, Eddie <eddie.dong@...>; acrn-dev@...
Subject: RE: [acrn-dev] [PATCH 1/2 v2] hv: shell: improve console to buffer
history cmds


Hi, Eddie,
Thanks for your comments!
Before the feature as follows:
1. it is a ring buffer, just store the new one if it is not same with last
one in buffer.

OK

2. just buffer the command after "enter" key

Fine, though it is different with bash.

I need re-consider how to refactor this feature after checking your
comments:
1. could combine input_line with buffered_cmds as one array.

For other comments, see inline answers.

Thanks
Minggui

-----Original Message-----
From: Dong, Eddie <eddie.dong@...>
Sent: Saturday, July 23, 2022 2:46 AM
To: acrn-dev@...
Cc: Cao, Minggui <minggui.cao@...>
Subject: RE: [acrn-dev] [PATCH 1/2 v2] hv: shell: improve console to
buffer history cmds

-----Original Message-----
From: acrn-dev@...
<acrn-dev@...> On Behalf Of Minggui Cao
Sent: Thursday, July 21, 2022 10:56 PM
To: acrn-dev@...
Cc: Cao, Minggui <minggui.cao@...>
Subject: [acrn-dev] [PATCH 1/2 v2] hv: shell: improve console to
buffer history cmds

1. buffer max to 8 history commands.
2. support up/down key to select history buffered commands

Signed-off-by: Minggui Cao <minggui.cao@...>
---
hypervisor/debug/shell.c | 140 ++++++++++++++++++++++++++++-----

-

hypervisor/debug/shell_priv.h | 9 +++
2 files changed, 127 insertions(+), 22 deletions(-)

diff --git a/hypervisor/debug/shell.c b/hypervisor/debug/shell.c
index
588e4aa0f..0bc982c27 100644
--- a/hypervisor/debug/shell.c
+++ b/hypervisor/debug/shell.c
@@ -158,6 +158,14 @@ static struct shell_cmd shell_cmds[] = {
},
};

+/* for function key: up/down key */ enum function_key {
+ KEY_NONE,
+
+ KEY_UP = 0x5B41,
+ KEY_DOWN = 0x5B42,
+};
+
/* The initial log level*/
uint16_t console_loglevel = CONFIG_CONSOLE_LOGLEVEL_DEFAULT;
uint16_t mem_loglevel = CONFIG_MEM_LOGLEVEL_DEFAULT; @@ -271,14
+279,89 @@ static uint16_t sanitize_vmid(uint16_t vmid)
return sanitized_vmid;
}

+static void clear_input_line(uint32_t len) {
+ while (len > 0) {
+ len--;
+ shell_puts("\b");
+ shell_puts(" \b");
+ }
+}
+static void copy_chars(char *dest, char *src, uint32_t size, bool
+from_head) {
+ uint32_t i = 0;
+
+ if (from_head) {
+ for (i = 0; i < size; i++) {
+ *(dest + i) = *(src + i);
+ }
+ } else {
+ for (i = size; i > 0; i--) {
+ *(dest + i - 1) = *(src + i - 1);
+ }
+ }
+}
+
+static void handle_updown_key(enum function_key key_value) {
+ uint32_t current_select = p_shell->selected_index;
+
+ if (p_shell->total_buffered_cmds == 0) {
+ return;
+ }
+
+ /* update p_shell->selected_index as up/down key */
+ if (key_value == KEY_UP) {
+
+ if ((p_shell->total_buffered_cmds < MAX_BUFFERED_CMDS)
&& (p_shell->selected_index > 0)) {

Didn't understand why we need to check "p_shell->total_buffered_cmds <
MAX_BUFFERED_CMDS" here.

[Cao, Minggui] it is a ring buffer, if the buffer is not full filled and full, the index
shall be Handled differently. For example, if total buffer size 8, now just
buffered 4, the index shall Be just in 3-0;

Understood. But we don't need to check it.

For key_UP, it can do like:
If ( --p_shell->selected_index < 0 )
p_shell->selected_index += p_shell->total_buffered_cmds;

+ p_shell->selected_index--;

.....

+ } else if (p_shell->total_buffered_cmds >=
MAX_BUFFERED_CMDS) {
+ uint32_t up_select = (p_shell->selected_index +
MAX_BUFFERED_CMDS -
+1) % MAX_BUFFERED_CMDS;

Don't understand why we need this conditional process. I think they
can be same.

[Cao, Minggui] if the ring buffer has been full, the index could be a full loop, It
need (p_shell->selected_index - 1 + MAX_BUFFERED_CMDS) %
MAX_BUFFERED_CMDS Not same as above.