Skip to content

Conversation

@CYFS3
Copy link
Contributor

@CYFS3 CYFS3 commented Dec 12, 2025

拉取/合并请求描述:(PR description)

[

为什么提交这份PR (why to submit this PR)

#10968

你的解决方案是什么 (what is your solution)

请提供验证的bsp和config (provide the config and bsp)

  • BSP:
  • .config:
  • action:

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/ALL_BSP_COMPILE.json 详细请参考链接BSP自查

@github-actions
Copy link

👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread!

为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。
To ensure your code complies with RT-Thread's coding style, please run the code formatting workflow by following the steps below (If the formatting of CI fails to run).


🛠 操作步骤 | Steps

  1. 前往 Actions 页面 | Go to the Actions page
    点击进入工作流 → | Click to open workflow →

  2. 点击 Run workflow | Click Run workflow

  • 设置需排除的文件/目录(目录请以"/"结尾)
    Set files/directories to exclude (directories should end with "/")
  • 将目标分支设置为 \ Set the target branch to:name_check
  • 设置PR number为 \ Set the PR number to:11042
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将自动推送至你的分支。
    The formatted code will be automatically pushed to your branch.

完成后,提交将自动更新至 name_check 分支,关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the name_check branch automatically, and the related Pull Request will be updated.

如有问题欢迎联系我们,再次感谢您的贡献!💐
If you have any questions, feel free to reach out. Thanks again for your contribution!

@github-actions github-actions bot added the Kernel PR has src relate code label Dec 12, 2025
@github-actions
Copy link

📌 Code Review Assignment

🏷️ Tag: kernel

Reviewers: @GorrayLi @ReviewSun @hamburger-os @lianux-mm @wdfk-prog @xu18838022837

Changed Files (Click to expand)
  • src/Kconfig
  • src/object.c
  • src/utest/object_tc.c

📊 Current Review Status (Last Updated: 2025-12-12 10:05 CST)


📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

@CYFS3
Copy link
Contributor Author

CYFS3 commented Jan 6, 2026

#11043

Copy link
Contributor

Copilot AI left a 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_CHECKS configuration option in Kconfig to enable strict validation
  • Implemented duplicate name detection and length overflow assertions in rt_object_init() and rt_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

Comment on lines +532 to +543
#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 */
Copy link

Copilot AI Jan 6, 2026

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) 来处理重复检查和长度验证。

Copilot uses AI. Check for mistakes.
rt_object_detach(&obj);
}

#if defined(RT_USING_STRICT_NAME_CHECKS) && defined(RT_DEBUGING_ASSERT)
Copy link

Copilot AI Jan 6, 2026

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 编码标准,函数应该有清晰的文档。考虑在此函数上方添加注释块,说明它测试严格名称检查功能,包括重复名称检测和长度溢出验证。

Suggested change
#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.
*/

Copilot uses AI. Check for mistakes.
Comment on lines +145 to +165
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;
}
Copy link

Copilot AI Jan 6, 2026

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 结构缺少文档注释。这些是内部测试工具,应该记录以解释它们在测试期间捕获断言调用的目的。添加注释说明此机制用于测试严格名称检查按预期触发断言。

Copilot uses AI. Check for mistakes.
Comment on lines +400 to +410
#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);
}
}
Copy link

Copilot AI Jan 6, 2026

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 帮助文本或代码注释中记录这种性能权衡,特别是因为这是用于开发时调试的。

Copilot uses AI. Check for mistakes.
Comment on lines +403 to +410
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);
}
}
Copy link

Copilot AI Jan 6, 2026

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。这是正确的行为,但在多线程场景中,如果两个线程在检查和实际注册之间同时尝试创建具有相同名称的对象,可能存在竞争条件。如果这对用例来说是一个问题,请考虑记录此限制或添加额外的保护。

Copilot uses AI. Check for mistakes.
#endif /* RT_USING_STRICT_NAME_CHECKS */

obj_name_len = rt_strlen(name);
if(obj_name_len > RT_NAME_MAX - 1)
Copy link

Copilot AI Jan 6, 2026

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 编码风格指南,该指南要求关键字和括号之间有空格。

Suggested change
if(obj_name_len > RT_NAME_MAX - 1)
if (obj_name_len > RT_NAME_MAX - 1)

Copilot uses AI. Check for mistakes.
Comment on lines +464 to +487
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);
Copy link

Copilot AI Jan 6, 2026

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_objoverflow_obj,即使它们的初始化失败(触发了断言)。这可能导致分离未初始化的对象。考虑在调用 detach 之前检查对象是否成功初始化,或验证 rt_object_detach 是否安全处理部分初始化的对象。

Copilot uses AI. Check for mistakes.
#ifdef RT_USING_STRICT_NAME_CHECKS
/* Strict name checks */
{
rt_object_t duplicate = rt_object_find(name, type);
Copy link
Member

@kurisaW kurisaW Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感觉这块弄得越来越繁琐了,传入的名称已经是确定的name,这里再查找一次感觉没有意义,更多还是要多次验证_match_name的健壮性

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Kernel PR has src relate code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants