-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[kennel][object]:Add strict name checks #11042
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-12-12 10:05 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 introduces strict name checking functionality for RT-Thread kernel objects to help developers detect configuration mistakes during development. The feature adds duplicate name detection and length validation that can trigger assertions when enabled.
Key Changes
- Added
RT_USING_STRICT_NAME_CHECKSconfiguration option in Kconfig to enable strict validation - Implemented duplicate name detection and length overflow assertions in
rt_object_init()andrt_object_allocate() - Added comprehensive unit tests to verify the strict checking behavior using assert hook capture
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| src/Kconfig | Adds new RT_USING_STRICT_NAME_CHECKS config option with help text explaining the feature |
| src/object.c | Implements strict name checking logic in object initialization and allocation functions with duplicate detection and length validation |
| src/utest/object_tc.c | Adds test infrastructure (assert capture mechanism) and comprehensive test function to validate strict name checking behavior |
| #ifdef RT_USING_STRICT_NAME_CHECKS | ||
| /* Strict name checks */ | ||
| { | ||
| rt_object_t duplicate = rt_object_find(name, type); | ||
|
|
||
| if (duplicate) | ||
| { | ||
| LOG_E("Object name %s already exists in type %d.", name, type); | ||
| RT_ASSERT(duplicate == RT_NULL); | ||
| } | ||
| } | ||
| #endif /* RT_USING_STRICT_NAME_CHECKS */ |
Copilot
AI
Jan 6, 2026
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.
English: The strict name check code is duplicated between rt_object_init (lines 400-410) and rt_object_allocate (lines 532-543). This duplicated logic should be extracted into a helper function to improve maintainability and reduce code duplication. Consider creating a function like rt_object_check_name_validity(const char *name, enum rt_object_class_type type, rt_size_t obj_name_len) to handle both duplicate checking and length validation.
中文:严格名称检查代码在 rt_object_init(第 400-410 行)和 rt_object_allocate(第 532-543 行)之间重复。应将此重复逻辑提取到辅助函数中以提高可维护性并减少代码重复。考虑创建一个函数,如 rt_object_check_name_validity(const char *name, enum rt_object_class_type type, rt_size_t obj_name_len) 来处理重复检查和长度验证。
| rt_object_detach(&obj); | ||
| } | ||
|
|
||
| #if defined(RT_USING_STRICT_NAME_CHECKS) && defined(RT_DEBUGING_ASSERT) |
Copilot
AI
Jan 6, 2026
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.
English: The new test function test_object_strict_name_checks lacks a documentation comment (docstring) to explain its purpose and behavior. According to RT-Thread coding standards, functions should have clear documentation. Consider adding a comment block above this function explaining that it tests the strict name checking feature including duplicate name detection and length overflow validation.
中文:新的测试函数 test_object_strict_name_checks 缺少文档注释(docstring)来解释其目的和行为。根据 RT-Thread 编码标准,函数应该有清晰的文档。考虑在此函数上方添加注释块,说明它测试严格名称检查功能,包括重复名称检测和长度溢出验证。
| #if defined(RT_USING_STRICT_NAME_CHECKS) && defined(RT_DEBUGING_ASSERT) | |
| #if defined(RT_USING_STRICT_NAME_CHECKS) && defined(RT_DEBUGING_ASSERT) | |
| /** | |
| * Test strict object name checking behavior. | |
| * | |
| * This test exercises the RT_USING_STRICT_NAME_CHECKS logic under | |
| * RT_DEBUGING_ASSERT by: | |
| * - Initializing a base object with a unique name. | |
| * - Attempting to initialize a second object with the same name to | |
| * verify duplicate name detection via the assertion hook. | |
| * - Constructing a name longer than TEST_RT_NAME_MAX to validate | |
| * name-length overflow handling. | |
| */ |
| struct strict_assert_capture | ||
| { | ||
| rt_bool_t armed; | ||
| rt_uint16_t hit_count; | ||
| const char *expr; | ||
| const char *func; | ||
| rt_size_t line; | ||
| }; | ||
|
|
||
| static struct strict_assert_capture strict_assert_capture_state; | ||
|
|
||
| static void strict_assert_hook(const char *ex, const char *func, rt_size_t line) | ||
| { | ||
| if (!strict_assert_capture_state.armed) | ||
| return; | ||
|
|
||
| strict_assert_capture_state.hit_count++; | ||
| strict_assert_capture_state.expr = ex; | ||
| strict_assert_capture_state.func = func; | ||
| strict_assert_capture_state.line = line; | ||
| } |
Copilot
AI
Jan 6, 2026
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.
English: The strict_assert_hook function and related strict_assert_capture structure lack documentation comments. These are internal test utilities that should be documented to explain their purpose in capturing assert calls during testing. Add comments explaining that this mechanism is used to test that the strict name checks trigger assertions as expected.
中文:strict_assert_hook 函数和相关的 strict_assert_capture 结构缺少文档注释。这些是内部测试工具,应该记录以解释它们在测试期间捕获断言调用的目的。添加注释说明此机制用于测试严格名称检查按预期触发断言。
| #ifdef RT_USING_STRICT_NAME_CHECKS | ||
| /* Strict name checks */ | ||
| { | ||
| rt_object_t duplicate = rt_object_find(name, type); | ||
|
|
||
| if (duplicate) | ||
| { | ||
| LOG_E("Object name %s already exists in type %d.", name, type); | ||
| RT_ASSERT(duplicate == RT_NULL); | ||
| } | ||
| } |
Copilot
AI
Jan 6, 2026
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.
English: The duplicate name check is performed inside rt_object_init, which could have performance implications. The rt_object_find function may need to iterate through the object list, adding overhead to every object initialization. Consider documenting this performance trade-off in the Kconfig help text or code comments, especially since this is meant for development-time debugging.
中文:在 rt_object_init 中执行重复名称检查可能会影响性能。rt_object_find 函数可能需要遍历对象列表,为每次对象初始化增加开销。考虑在 Kconfig 帮助文本或代码注释中记录这种性能权衡,特别是因为这是用于开发时调试的。
| rt_object_t duplicate = rt_object_find(name, type); | ||
|
|
||
| if (duplicate) | ||
| { | ||
| LOG_E("Object name %s already exists in type %d.", name, type); | ||
| RT_ASSERT(duplicate == RT_NULL); | ||
| } | ||
| } |
Copilot
AI
Jan 6, 2026
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.
English: The duplicate name check calls rt_object_find before the object is added to the object list. This is correct behavior, but there's a potential race condition in multi-threaded scenarios if two threads try to create objects with the same name simultaneously between the check and the actual registration. Consider documenting this limitation or adding additional protection if this is a concern for the use case.
中文:重复名称检查在对象添加到对象列表之前调用 rt_object_find。这是正确的行为,但在多线程场景中,如果两个线程在检查和实际注册之间同时尝试创建具有相同名称的对象,可能存在竞争条件。如果这对用例来说是一个问题,请考虑记录此限制或添加额外的保护。
| #endif /* RT_USING_STRICT_NAME_CHECKS */ | ||
|
|
||
| obj_name_len = rt_strlen(name); | ||
| if(obj_name_len > RT_NAME_MAX - 1) |
Copilot
AI
Jan 6, 2026
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.
English: Missing space after 'if' keyword. Should be if (obj_name_len > RT_NAME_MAX - 1) to comply with RT-Thread coding style guide which requires a space between keywords and parentheses.
中文:if 关键字后缺少空格。应该是 if (obj_name_len > RT_NAME_MAX - 1) 以符合 RT-Thread 编码风格指南,该指南要求关键字和括号之间有空格。
| if(obj_name_len > RT_NAME_MAX - 1) | |
| if (obj_name_len > RT_NAME_MAX - 1) |
| rt_object_init(&duplicate_obj, RT_Object_Class_Thread, duplicate_name); | ||
| uassert_true(strict_assert_capture_state.hit_count >= 1); | ||
| uassert_not_null(strict_assert_capture_state.expr); | ||
| uassert_str_equal(strict_assert_capture_state.expr, "duplicate == RT_NULL"); | ||
| strict_assert_capture_state.armed = RT_FALSE; | ||
| rt_object_detach(&duplicate_obj); | ||
|
|
||
| uassert_true(generate_unique_name(overflow_name, sizeof(overflow_name), "strict", RT_Object_Class_Thread) == RT_EOK); | ||
| base_len = rt_strlen(overflow_name); | ||
| rt_memset(overflow_name + base_len, 'x', sizeof(overflow_name) - base_len - 1); | ||
| overflow_name[sizeof(overflow_name) - 1] = '\0'; | ||
| uassert_true(rt_strlen(overflow_name) > TEST_RT_NAME_MAX - 1); | ||
|
|
||
| strict_assert_capture_state.hit_count = 0; | ||
| strict_assert_capture_state.expr = RT_NULL; | ||
| strict_assert_capture_state.func = RT_NULL; | ||
| strict_assert_capture_state.line = 0; | ||
| strict_assert_capture_state.armed = RT_TRUE; | ||
| rt_object_init(&overflow_obj, RT_Object_Class_Thread, overflow_name); | ||
| uassert_true(strict_assert_capture_state.hit_count >= 1); | ||
| uassert_not_null(strict_assert_capture_state.expr); | ||
| uassert_str_equal(strict_assert_capture_state.expr, "obj_name_len <= RT_NAME_MAX - 1"); | ||
| strict_assert_capture_state.armed = RT_FALSE; | ||
| rt_object_detach(&overflow_obj); |
Copilot
AI
Jan 6, 2026
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.
English: The test function properly validates the strict name check feature by capturing assert calls. However, it detaches the duplicate_obj and overflow_obj even though their initialization failed (assert was triggered). This could lead to detaching uninitialized objects. Consider checking if the object was successfully initialized before calling detach, or verify that rt_object_detach handles partially initialized objects safely.
中文:测试函数通过捕获断言调用正确验证了严格名称检查功能。但是,它会分离 duplicate_obj 和 overflow_obj,即使它们的初始化失败(触发了断言)。这可能导致分离未初始化的对象。考虑在调用 detach 之前检查对象是否成功初始化,或验证 rt_object_detach 是否安全处理部分初始化的对象。
| #ifdef RT_USING_STRICT_NAME_CHECKS | ||
| /* Strict name checks */ | ||
| { | ||
| rt_object_t duplicate = rt_object_find(name, type); |
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.
感觉这块弄得越来越繁琐了,传入的名称已经是确定的name,这里再查找一次感觉没有意义,更多还是要多次验证_match_name的健壮性
拉取/合并请求描述:(PR description)
[
为什么提交这份PR (why to submit this PR)
#10968
你的解决方案是什么 (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