[Feature][Shell] 增加对 Home、Insert、Delete 和 End 键的支持，改进输入模式处理 #10595
Conversation
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-08-12 09:18 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for Home, Insert, Delete, and End key functionality to the finsh shell component, along with improved input mode handling. The changes enhance the shell's text editing capabilities by implementing standard terminal key bindings and introducing an overwrite/insert mode toggle.
- Adds new
WAIT_EXT_KEYstate to handle extended key sequences for Home, Insert, Delete, and End keys - Implements an overwrite mode toggle that can be controlled via the Insert key
- Refactors character insertion logic to properly handle both insert and overwrite modes
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| components/finsh/shell.h | Adds new input state WAIT_EXT_KEY and overwrite_mode flag to shell structure |
| components/finsh/shell.c | Implements extended key handling logic and improved character insertion with mode support |
| shell->stat = WAIT_EXT_KEY; | ||
| shell->line[shell->line_position + 1] = ch; /* store the key code */ |
There was a problem hiding this comment.
Storing the key code at shell->line_position + 1 could cause buffer overflow or corrupt user input data. The shell->line buffer is used for command input and overwriting it with key codes can lead to unexpected behavior. Consider using a separate temporary variable to store the key code.
| shell->stat = WAIT_EXT_KEY; | |
| shell->line[shell->line_position + 1] = ch; /* store the key code */ | |
| shell->stat = WAIT_EXT_KEY; | |
| shell->ext_key_code = ch; /* store the key code in a dedicated variable */ |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
新增一个没必要 在line的缓存区申请中就已经有个多余的字节放在了最后 可以利用起来 不会产生越界操作
|
|
||
| if (ch == 0x7E) /* extended key terminator */ | ||
| { | ||
| rt_uint8_t key_code = shell->line[shell->line_position + 1]; |
There was a problem hiding this comment.
Reading the key code from shell->line_position + 1 is problematic because this location in the buffer may contain user input data or be out of bounds. This creates a dependency on the previous problematic storage at line 686 and could lead to incorrect key code interpretation.
| rt_uint8_t key_code = shell->line[shell->line_position + 1]; | |
| rt_uint8_t key_code = shell_ext_key_code; |
Copilot uses AI. Check for mistakes.
components/finsh/shell.c
Outdated
| /* move existing characters to the right */ | ||
| rt_memmove(&shell->line[shell->line_curpos + 1], | ||
| &shell->line[shell->line_curpos], | ||
| shell->line_position - shell->line_curpos + 1); |
There was a problem hiding this comment.
Adding + 1 to the memmove length could cause buffer overflow. The original calculation shell->line_position - shell->line_curpos correctly represents the number of characters to move. The extra + 1 could copy beyond the intended range and potentially access uninitialized memory or overwrite the null terminator.
| shell->line_position - shell->line_curpos + 1); | |
| shell->line_position - shell->line_curpos); |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
shell->line_position是有效数据长度 但是缺少字符串结束符'\0' 因此需要+1
meng-plus
force-pushed
the
feature/shell
branch
from
f61ba30 to
a0f81a0
Compare
|
作者可以看一下这个 #10394 (comment) |
对比看了一下 你那边属于增加功能 我补充的这点应该归属于基础操作 del ins都不支持 用起来 太难受了 |
CI问题需要处理一下 |
meng-plus
force-pushed
the
feature/shell
branch
from
a0f81a0 to
9ad2a45
Compare
meng-plus
force-pushed
the
feature/shell
branch
from
9ad2a45 to
8d944bc
Compare
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
补全交互操作
你的解决方案是什么 (what is your solution)
增加交互处理逻辑
请提供验证的bsp和config (provide the config and bsp)
]
当前拉取/合并请求的状态 Intent for your PR
必须选择一项 Choose one (Mandatory):
代码质量 Code Quality：
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码，不包含已经被注释了的代码 All redundant code is removed and cleaned up