[rt-link] fix the compiling issue under 64bit arch#11018
[rt-link] fix the compiling issue under 64bit arch#11018Rbb666 merged 2 commits intoRT-Thread:masterfrom
Conversation
|
👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。 🛠 操作步骤 | Steps
完成后,提交将自动更新至 如有问题欢迎联系我们,再次感谢您的贡献!💐 |
📌 Code Review Assignment🏷️ Tag: componentsReviewers: Maihuanyi Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-12-04 21:28 CST)
📝 Review Instructions
|
There was a problem hiding this comment.
Pull request overview
This PR fixes compilation issues in the rt-link component when building for 64-bit architectures. The changes ensure type safety by using architecture-agnostic types (rt_ubase_t, ssize_t) instead of hardcoded 32-bit types, addressing pointer size and data type mismatches on 64-bit platforms.
Key Changes:
- Converted timer parameter type casts from
rt_uint32_ttort_ubase_tfor 64-bit pointer compatibility - Updated DFS file operation function signatures to use
ssize_treturn type per standard interface requirements - Increased thread stack size from 1KB to 4KB to accommodate 64-bit stack frame requirements
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
components/utilities/rt-link/src/rtlink_dev.c |
Added explicit includes for dfs_file.h and fcntl.h; changed read/write function return types and local variables from int to ssize_t for 64-bit compatibility with DFS file operations interface |
components/utilities/rt-link/src/rtlink.c |
Changed timer parameter handling from rt_uint32_t to rt_ubase_t casts (5 locations); increased thread stack size from 1KB to 4KB; reordered hardware deinitialization before critical section entry; added error checking for rt_link_hw_init() |
| { | ||
| rt_enter_critical(); | ||
| rt_link_hw_deinit(); | ||
| rt_enter_critical(); |
There was a problem hiding this comment.
[Concurrency/并发] Potential race condition in critical section reordering / 临界区重排可能引入竞争条件
English: Moving rt_link_hw_deinit() before rt_enter_critical() creates a potential race condition. If another thread accesses rt_link_scb between rt_link_hw_deinit() and rt_enter_critical(), it may observe inconsistent state.
Consider this ordering instead:
- Enter critical section first
- Save a local copy of
rt_link_scb - Set
rt_link_scb = RT_NULL - Exit critical section
- Call
rt_link_hw_deinit()and other cleanup using the local copy
中文:将 rt_link_hw_deinit() 移到 rt_enter_critical() 之前可能引入竞争条件。如果其他线程在 rt_link_hw_deinit() 和 rt_enter_critical() 之间访问 rt_link_scb,可能会观察到不一致的状态。
建议采用以下顺序:
- 先进入临界区
- 保存
rt_link_scb的本地副本 - 设置
rt_link_scb = RT_NULL - 退出临界区
- 使用本地副本调用
rt_link_hw_deinit()和其他清理操作
| #define RT_LINK_THREAD_TICK 20 | ||
| #define RT_LINK_THREAD_PRIORITY 25 | ||
| #define RT_LINK_THREAD_STACK_SIZE 1024 /* 32 bytes aligned */ | ||
| #define RT_LINK_THREAD_STACK_SIZE (4 * 1024) /* 32 bytes aligned */ |
There was a problem hiding this comment.
[Maintainability/可维护性] Stack size increase needs justification / 栈大小增加需要说明理由
English: The stack size is increased from 1024 to 4096 bytes (4x increase). While this may be necessary for 64-bit architectures due to larger pointer sizes and stack frame alignment requirements, the change lacks documentation explaining:
- Why this specific size (4KB) was chosen
- Whether this has been tested on resource-constrained devices
- The actual stack usage observed on 64-bit platforms
Consider adding a comment explaining the rationale, or using conditional compilation to set different stack sizes for 32-bit vs 64-bit architectures:
#ifdef ARCH_CPU_64BIT
#define RT_LINK_THREAD_STACK_SIZE (4 * 1024) /* Larger stack for 64-bit */
#else
#define RT_LINK_THREAD_STACK_SIZE 1024 /* Original size for 32-bit */
#endif中文:栈大小从 1024 字节增加到 4096 字节(增加了 4 倍)。虽然这对于 64 位架构可能是必要的(由于更大的指针大小和栈帧对齐要求),但此更改缺少文档说明:
- 为什么选择这个特定大小(4KB)
- 是否在资源受限的设备上进行了测试
- 在 64 位平台上观察到的实际栈使用情况
建议添加注释说明理由,或使用条件编译为 32 位和 64 位架构设置不同的栈大小(见示例代码)。
| #define RT_LINK_THREAD_STACK_SIZE (4 * 1024) /* 32 bytes aligned */ | |
| /* | |
| * RT_LINK_THREAD_STACK_SIZE: | |
| * For 64-bit architectures, stack frames and pointers are larger, requiring more stack space. | |
| * For 32-bit architectures, 1024 bytes is sufficient for typical usage. | |
| * If you are running on a resource-constrained device, consider profiling actual stack usage. | |
| */ | |
| #ifdef ARCH_CPU_64BIT | |
| #define RT_LINK_THREAD_STACK_SIZE (4 * 1024) /* Larger stack for 64-bit arch */ | |
| #else | |
| #define RT_LINK_THREAD_STACK_SIZE 1024 /* Original size for 32-bit arch */ | |
| #endif |
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
rt-link在64位系统上构建会存在问题;
你的解决方案是什么 (what is your solution)
在类型转换上使用更通用的rt_base_t/rt_ubase_t,而不是强制32位。
请提供验证的bsp和config (provide the config and bsp)
qemu-virt64-aarch64/qemu-virt64-riscv
]
当前拉取/合并请求的状态 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