fix[object]: 修复rt_object_find当finsh_set_device(console->parent.name);错误 #8964
Conversation
|
感觉 #8959 的修复太复杂了，这里给出一个我的方案 |
src/object.c
Outdated
| @@ -620,6 +620,7 @@ rt_err_t rt_object_for_each(rt_uint8_t type, rt_object_iter_t iter, void *data) | |||
| } | |||
| } | |||
|
|
|||
| *(rt_object_t *)data = NULL; | |||
There was a problem hiding this comment.
😂 这个并不是专门为了 rt_object_find 设计的 API，而是一个公共 API。这么改 data 就必须是一个指向 rt_object_t 的指针？
其他场景传来一个 RT_NULL，或者别的具有其它意义的数字怎么办。
There was a problem hiding this comment.
那就不改类型了 只修改 *data = NULL, 这个函数的设计上复用了data 让其传name进来又传结果出去，走到623行，传入name的任务已经完成，如果此处不进行清理那就是错误的传出去一个object对象，虽然外面的做了一个 if (data != name)来过滤掉 找不到object的情况， 明显当前发现它并能正确的解决我们的需求，不如在退出函数前，将data修正NULL 避免外层做第二次判断
There was a problem hiding this comment.
首先，这没有解决 data == RT_NULL，data == (int)0x12344321 的场景。作为 iterator，对于 data 的实际 value 是不能做出任何假定的。
第二，从代码质量来说复用同一个变量做多件事情是不合理的。对于 reviewer 来说，难以理解含义也难以维护。这个修复只是把上个问题的直接现象解决了，却没有考虑到更深层次的问题，和核心矛盾。
There was a problem hiding this comment.
您说的 没错， rt_object_for_each的返回值并没有被很好的利用 能否考虑用它来做命中判定
There was a problem hiding this comment.
这个返回值最好就是只作为 rt_object_for_each 的返回值了，如果迭代器需要返回值，理应也是通过 data 传入一个自定义的对象，里面再包括一个 errno 的成员。不然这个东西越写越混淆，太容易误会了 😢
There was a problem hiding this comment.
我了解设计意图后，又进行了调整，感觉这样舒服点了，
rt_err_t rt_object_for_each(rt_uint8_t type, rt_object_iter_t iter, void *data)
{
struct rt_object *object = RT_NULL;
struct rt_list_node *node = RT_NULL;
struct rt_object_information *information = RT_NULL;
rt_base_t level;
rt_err_t error;
information = rt_object_get_information((enum rt_object_class_type)type);
/* parameter check */
if (information == RT_NULL)
{
return -RT_EINVAL;
}
/* which is invoke in interrupt status */
RT_DEBUG_NOT_IN_INTERRUPT;
/* enter critical */
level = rt_spin_lock_irqsave(&(information->spinlock));
/* try to find object */
rt_list_for_each(node, &(information->object_list))
{
object = rt_list_entry(node, struct rt_object, list);
if ((error = iter(object, data)) != RT_EOK)
{
rt_spin_unlock_irqrestore(&(information->spinlock), level);
return error >= 0 ? RT_EOK : error;
}
}
rt_spin_unlock_irqrestore(&(information->spinlock), level);
return -RT_ERROR; //修改了这里
}There was a problem hiding this comment.
如果进一步把 list_object 改用 iterator 的形式来实现的话，全部遍历一遍也应该是正常返回 RT_EOK 的。
总之 iter 才是真正知道迭代含义的，在外面包含太多使用场景的假定感觉都是不合理的。
fix[object.c] 修复&object->name == object 时rt_object_find失效
拉取/合并请求描述：(PR description)
[
为什么提交这份PR (why to submit this PR)
#8960
你的解决方案是什么 (what is your solution)
在
rt_object_for_each(type, _match_name, &data);中当搜索不到时 data清空为NULL请提供验证的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