-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[kernel]fix thread error code to avoid spurious error display in list_thread. #10972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。 🛠 操作步骤 | Steps
完成后,提交将自动更新至 如有问题欢迎联系我们,再次感谢您的贡献!💐 |
📌 Code Review Assignment🏷️ Tag: kernelReviewers: GorrayLi ReviewSun hamburger-os lianux-mm wdfk-prog xu18838022837 Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-11-23 22:35 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes issue #6768 by introducing a sentinel error code RT_EWAITPEND to avoid spurious error displays in the list_thread command. The solution prevents temporary waiting states from being shown as errors by using an internal sentinel value that gets normalized to the appropriate error code after thread scheduling.
Key changes:
- Introduces
RT_EWAITPENDas an internal sentinel error code mapped to "OK" in error string display - Adds
_ipc_wait_result()helper function to normalize sentinel values back toRT_EINTR - Updates all IPC wait operations (semaphore, mutex, event, mailbox, message queue) and thread sleep to use the sentinel pattern
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| include/klibc/kerrno.h | Defines RT_EWAITPEND error code (19 for non-libc, EINPROGRESS for libc builds) |
| src/klibc/kerrno.c | Maps RT_EWAITPEND to "OK" string to avoid spurious error display |
| src/thread.c | Updates thread sleep to use RT_EWAITPEND sentinel before scheduling |
| src/ipc.c | Adds _ipc_wait_result() normalization function and updates all IPC wait operations to use RT_EWAITPEND sentinel pattern |
src/ipc.c
Outdated
| * When the return value is RT_EINTR, the operation was interrupted (interrupted system call). | ||
| * When the return value is RT_ETIMEOUT, the operation timed out. | ||
| * | ||
| * @see _ipc_mark_thread_waiting() |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Documentation/文档]: Invalid function reference in @see tag / @see 标签中的无效函数引用
English: The @see _ipc_mark_thread_waiting() reference is invalid - this function does not exist in the codebase. Please remove this line or update it to reference a valid function.
中文:@see _ipc_mark_thread_waiting() 引用无效 - 此函数在代码库中不存在。请删除此行或更新为有效的函数引用。
| * @see _ipc_mark_thread_waiting() |
| #define RT_ENOBUFS 16 /**< No buffer space is available */ | ||
| #define RT_ESCHEDISR 17 /**< scheduler failure in isr context */ | ||
| #define RT_ESCHEDLOCKED 18 /**< scheduler failure in critical region */ | ||
| #define RT_EWAITPEND 19 /**< Wait operation pending */ |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Documentation/文档]: Inconsistent documentation between LIBC and non-LIBC builds / LIBC 和非 LIBC 构建之间的文档不一致
English: The documentation comment for RT_EWAITPEND is inconsistent between the two definitions. Line 36 says "Wait operation pending (internal sentinel)" while line 63 only says "Wait operation pending". Please add "(internal sentinel)" to line 63's comment for consistency, as this is a crucial detail indicating it's not meant to be exposed to users.
中文:RT_EWAITPEND 在两个定义之间的文档注释不一致。第 36 行说"Wait operation pending (internal sentinel)",而第 63 行只说"Wait operation pending"。请在第 63 行的注释中添加"(internal sentinel)"以保持一致性,因为这是一个关键细节,表明它不应向用户公开。
Suggested/建议:
#define RT_EWAITPEND 19 /**< Wait operation pending (internal sentinel) */| #define RT_EWAITPEND 19 /**< Wait operation pending */ | |
| #define RT_EWAITPEND 19 /**< Wait operation pending (internal sentinel) */ |
src/ipc.c
Outdated
| * the thread was interrupted by an interrupted system call rather than successfully | ||
| * acquiring the resource or timing out. |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Spelling/拼写]: Typo in comment / 注释中的拼写错误
English: "interrupted system call" is redundant. The phrase should be "interrupted" since EINTR already means "interrupted system call". Alternatively, you could say "the thread was interrupted" or "the operation was interrupted by a signal".
中文:"interrupted system call" 有冗余。应该使用 "interrupted",因为 EINTR 本身就表示"中断的系统调用"。或者可以说"the thread was interrupted"或"the operation was interrupted by a signal"。
Original: "the thread was interrupted by an interrupted system call"
Suggested/建议: "the thread was interrupted" or "the operation was interrupted by a signal"
| * the thread was interrupted by an interrupted system call rather than successfully | |
| * acquiring the resource or timing out. | |
| * the thread was interrupted rather than successfully acquiring the resource or timing out. | |
| * (i.e. the operation was interrupted by a signal) |
src/ipc.c
Outdated
| * 2022-10-16 Bernard add prioceiling feature in mutex | ||
| * 2023-04-16 Xin-zheqi redesigen queue recv and send function return real message size | ||
| * 2023-09-15 xqyjlj perf rt_hw_interrupt_disable/enable | ||
| * 2025-11-23 Rbb666 fix thread error code to avoid spurious error display in ps |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Spelling/拼写]: Typo in changelog comment / 变更日志注释中的拼写错误
English: "ps" should be "list_thread" to match the PR title and description. The issue is about spurious error display in list_thread command, not ps command.
中文:应将 "ps" 改为 "list_thread" 以匹配 PR 标题和描述。该问题是关于 list_thread 命令中的错误显示,而不是 ps 命令。
Original: "fix thread error code to avoid spurious error display in ps"
Corrected/更正: "fix thread error code to avoid spurious error display in list_thread"
| * 2025-11-23 Rbb666 fix thread error code to avoid spurious error display in ps | |
| * 2025-11-23 Rbb666 fix thread error code to avoid spurious error display in list_thread |
src/ipc.c
Outdated
| * the thread was interrupted by an interrupted system call rather than successfully | ||
| * acquiring the resource or timing out. |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Documentation/文档]: Inaccurate function documentation / 函数文档不准确
English: The documentation states "the thread was interrupted by an interrupted system call rather than successfully acquiring the resource or timing out", but this is confusing. It should clarify that RT_EWAITPEND indicates the thread is still in a waiting state and hasn't been woken up by resource acquisition or timeout. When it remains RT_EWAITPEND after scheduling, it means the thread was woken by a signal/interrupt instead.
中文:文档说"the thread was interrupted by an interrupted system call rather than successfully acquiring the resource or timing out",但这令人困惑。应该澄清 RT_EWAITPEND 表示线程仍处于等待状态,尚未被资源获取或超时唤醒。当调度后仍为 RT_EWAITPEND 时,表示线程是被信号/中断唤醒的。
Suggested/建议:
* @note This function translates the internal RT_EWAITPEND sentinel value back to RT_EINTR
* for the caller. If the error code is still RT_EWAITPEND after scheduling, it means
* the thread was woken by a signal rather than by successfully acquiring the resource
* or timing out.| * the thread was interrupted by an interrupted system call rather than successfully | |
| * acquiring the resource or timing out. | |
| * the thread is still in a waiting state and hasn't been woken up by resource acquisition or timeout. | |
| * When it remains RT_EWAITPEND after scheduling, it means the thread was woken by a signal/interrupt instead. |
src/ipc.c
Outdated
| /* unwrap sentinel: signal -> -RT_EINTR, timeout already set */ | ||
| /* convert sentinel back to EINTR/timeout semantics */ | ||
| /* translate sentinel back to EINTR or timeout */ |
Copilot
AI
Nov 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Maintainability/可维护性]: Redundant comments / 冗余注释
English: Four consecutive comments on lines 2239-2242 are saying the same thing. Please keep only one clear comment.
中文:2239-2242 行的四个连续注释表达了相同的意思。请只保留一个清晰的注释。
Suggested/建议:
/* normalize sentinel before reporting to caller */
ret = _ipc_wait_result(thread);| /* unwrap sentinel: signal -> -RT_EINTR, timeout already set */ | |
| /* convert sentinel back to EINTR/timeout semantics */ | |
| /* translate sentinel back to EINTR or timeout */ |
32aeba5 to
76325bf
Compare
76325bf to
7d74213
Compare
| {RT_ENOENT , "ENOENT "}, /**< No such file or directory. */ | ||
| {RT_ENOSPC , "ENOSPC "}, /**< No space left on device. */ | ||
| {RT_EPERM , "EPERM "}, /**< Operation not permitted. */ | ||
| {RT_EWAITPEND, "OK "}, /**< Wait pending, internal sentinel for scheduler. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 这个显示成"ok"是不是不大好,显示成"WAITPEND".这样准确的?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
那这个PR貌似意义就不大了hh,这个PR就是修复ps的时候显示错误状态的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
应该是有一个比较好的状态信息显示。
| * Note: assert to avoid an unexpected resume | ||
| * Note: clear any temporary error from interruptible wait | ||
| */ | ||
| RT_ASSERT(thread->error == RT_EOK); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里应该保留原来的逻辑
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
修复如下issue的问题:
#6768
你的解决方案是什么 (what is your solution)
添加哨兵标记线程状态,用于检测线程的状态码是否被IPC/信号的唤醒机制修改
请提供验证的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