-
Notifications
You must be signed in to change notification settings - Fork 5.3k
fix(kernel)/improve(utest):fix the legacy issue related to the length of the object name version #10537
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
Conversation
📌 Code Review Assignment🏷️ Tag: kernelReviewers: GorrayLi ReviewSun hamburger-os lianux-mm wdfk-prog xu18838022837 Changed Files (Click to expand)
🏷️ Tag: workflowReviewers: Rbb666 kurisaW supperthomas Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-07-30 10:36 CST)
📝 Review Instructions
|
8bdbbce to
9263b8a
Compare
… of the object name version
|
如果这种传入的字符串超过 建议:直接在对象注册那边卡一下,如果名字超过长度,直接注册设备失败,给个失败警告,然后返回。 |
87ed8fd to
1c61ab2
Compare
Fixes: 62f3fb4: fix(kernel)/improve(utest):fix the legacy issue related to the length of the object name version RT-Thread#10537 After this patch, if length of object name exceeds (RT_NAME_MAX - 1), RTT will assert and oops when runing, but not in period of building. Though I don't think it's a good solution, but don't want to argue more about this. Old RT_NAME_MAX is 8 for k230, and some object names, such as "hwtimer0", which name length is 8, breaking the new rule. Just update configuration of k230 bsp and increase RT_NAME_MAX from 8 to 16, which should be long enough for k230. Signed-off-by: Chen Wang <[email protected]>
Fixes: 62f3fb4: fix(kernel)/improve(utest):fix the legacy issue related to the length of the object name version RT-Thread#10537 After this patch, if length of object name exceeds (RT_NAME_MAX - 1), RTT will assert and oops when runing, but not in period of building. Though I don't think it's a good solution, but don't want to argue more about this. Old RT_NAME_MAX is 8 for k230, and some object names, such as "hwtimer0", which name length is 8, breaking the new rule. Just update configuration of k230 bsp and increase RT_NAME_MAX from 8 to 16, which should be long enough for k230. Signed-off-by: Chen Wang <[email protected]>
Fixes: 62f3fb4: fix(kernel)/improve(utest):fix the legacy issue related to the length of the object name version #10537 After this patch, if length of object name exceeds (RT_NAME_MAX - 1), RTT will assert and oops when runing, but not in period of building. Though I don't think it's a good solution, but don't want to argue more about this. Old RT_NAME_MAX is 8 for k230, and some object names, such as "hwtimer0", which name length is 8, breaking the new rule. Just update configuration of k230 bsp and increase RT_NAME_MAX from 8 to 16, which should be long enough for k230. Signed-off-by: Chen Wang <[email protected]>
Fixes: 62f3fb4: fix(kernel)/improve(utest):fix the legacy issue related to the length of the object name version RT-Thread#10537 After this patch, if length of object name exceeds (RT_NAME_MAX - 1), RTT will assert and oops when runing, but not in period of building. Though I don't think it's a good solution, but don't want to argue more about this. Old RT_NAME_MAX is 8 for k230, and some object names, such as "hwtimer0", which name length is 8, breaking the new rule. Just update configuration of k230 bsp and increase RT_NAME_MAX from 8 to 16, which should be long enough for k230. Signed-off-by: Chen Wang <[email protected]>
Fixes: 62f3fb4: fix(kernel)/improve(utest):fix the legacy issue related to the length of the object name version RT-Thread#10537 After this patch, if length of object name exceeds (RT_NAME_MAX - 1), RTT will assert and oops when runing, but not in period of building. Though I don't think it's a good solution, but don't want to argue more about this. Old RT_NAME_MAX is 8 for k230, and some object names, such as "hwtimer0", which name length is 8, breaking the new rule. Just update configuration of k230 bsp and increase RT_NAME_MAX from 8 to 16, which should be long enough for k230. Signed-off-by: Chen Wang <[email protected]>
Fixes: 62f3fb4: fix(kernel)/improve(utest):fix the legacy issue related to the length of the object name version RT-Thread#10537 After this patch, if length of object name exceeds (RT_NAME_MAX - 1), RTT will assert and oops when runing, but not in period of building. Though I don't think it's a good solution, but don't want to argue more about this. Old RT_NAME_MAX is 8 for k230, and some object names, such as "hwtimer0", which name length is 8, breaking the new rule. Just update configuration of k230 bsp and increase RT_NAME_MAX from 8 to 16, which should be long enough for k230. Signed-off-by: Chen Wang <[email protected]>
|
这里改动还是太激进了,感觉还是加入兼容模式比较好。 最不济修改下 406行和522行 rt_memcpy的写法,保证不会发生访问越界吧 我也是发现打印线程信息的时候,有几个线程信息丢失了,才发现这里内存访问越界了,还是很可怕的 |
对于兼容模式的话,大佬有什么好的想法吗,或许可以提个PR? |
|
现在强制触发 |
|
assert应该在编译阶段就会截停RT_NAME_MAX问题吧,我觉得这应该不算很大问题,按照此处的逻辑,RT_NAME_MAX 本就是RT-Thread系统中对于对象定义的最大限制,用户要么选择名字长度设置小一点,要么选择扩大RT_NAME_MAX,如果还保留之前的模式的话,那么在查找对象的时候,一旦超出RT_NAME_MAX 限制,会存在查找错误的情况,这本身也不合理,已经有开发者出现踩坑情况了 另外如果说真的考虑说资源占用,会有一个比较大的数组,我觉得可以考虑说加入动态适应创建对象名称这个机制 |
|
问题就是不会编译期报错,要到运行时。 而且如果对象不是开机就创建而是特定条件才会创建,就更难发现了 |
|
动态创建应该很难,rt_malloc还有头部空间,反而占用可能会更高 |
|
您说的坑点,主要是因为之前没有任何提示开发者溢出了,现在已经明确提示了,发现ERROR错误肯定会进行修改或者更容易排查问题了。 或者有什么办法能在编译时就能检查出所有对象名称有没有超界,这样保持现在这样是没问题的,强制用户必须解决。 应该没办法编译时发现,还一些组件和软件包对象的名称是使用rt_snprintf拼接的 |
理解你的意思了,这样确实不会在程序运行之前就发现问题,另外考虑是用户可能没有开启assert检查还有LOG打印是吧,是不是换成rt_kprintf打印会好点,至少提示信息能打印出来,然后引导用户去修改大小,我能想出比较好的就是日志提示一下 |
|
继续使用之前截断方式,超长加个日志提示。 |
|
好像"sys workq"这样子的直接就报断言了,每个bsp给改过来也比较麻烦吧,搜了下很多都是8。系统自带的这些要不检查一下改下名缩到8以内吧,要不跑bsp直接出断言错误还是比较烦的。 |
|
现在才看到这个pr,我感觉太激进了吧,本来这个名字就没说一定要\0结尾,现在假如系统里有一个对象名字很长,就要所有对象占用的空间平白多那么多,这太不友好了,就我实际项目经验来说,这个名字基本没什么用,反而每次创建多个对象的时候,都为这个名字要搞半天,我甚至希望有创建匿名对象的API,省功夫省内存空间,如果担心重名并且需要在代码中去做匹配,那完全可以想办法通过别的方式去匹配,或者内核配置时添加宏开关来控制,毕竟内核本来就没有判断对象是否重名。 |
支持+1 我说咋上周使用最新内核的项目中跑不起来,后面与能跑起来的版本全文对比,发现仅是NAME_MAX配置不同。。。。 其实有个想法,不一定实施,可以研究探讨下: 现在不同的项目中NAME_MAX不同会造成sizeof差异,有时项目发展过程中还多次调大这个配置, 如果这里的name改为指针,就没这个问题了。但是name也需要额外的存储空间,如果是malloc就更费,常见的是跟在obj之后。 进一步优化,这个指针目前至少为32位,可以用其最高2位来表示这是指针还是直接就是数据,甚至可以用上int压缩算法。 如果名称超过4,就使用指针方式指向目标。这样再长的名称也能放得下。不再需要 NAME_MAX 这个配置项了。 对应地,64位同样适用,对应提升一下即可。 |
|
感谢大家的反馈,这个PR和开源社区这块同步的确实不足,下周我们会召开一次对外例会,同步一下最新主线v5.2.2版本的更新,同时梳理一下一些比较有争议的PR吧,想要参会的可以邮箱通知我一下:[email protected] |
+1 |
用指针的话假如真的需要写入名称感觉对空间浪费更严重了,内存分配出来的对象的头部空间占用太多了,名字短了的话空间利用率很低,并且现有代码中所有对象都是具名的,那样老代码的空间内存占用就上来了,而且还要考虑没有使能内存动态分配的情况,比较麻烦。 我之前的想法是重新添加一些创建匿名对象的API,比如rt_mutex_create_anonymous这样的API。 如果大家都觉得这是个痛点的话,我倒是觉得可以好好讨论讨论。 |
|
@dongly @Ryan-CW-Code @kaidegit @milo-9 @armink @aozima 几位大佬可以通过下面的会议链接参加下本周日上午的社区对外例会哈,届时可以一起讨论下此PR问题 会议主题:RT-Thread社区讨论会 点击链接入会,或添加至会议列表: #腾讯会议:109-998-833 |


拉取/合并请求描述:(PR description)
[
Fixed: #10528
feature:object name超过RT_NAME_MAX-1,返回断言
improve:完善object测试用例
为什么提交这份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