-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😂 这个并不是专门为了 rt_object_find
设计的 API,而是一个公共 API。这么改 data 就必须是一个指向 rt_object_t 的指针?
其他场景传来一个 RT_NULL,或者别的具有其它意义的数字怎么办。
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.
那就不改类型了 只修改 *data = NULL
, 这个函数的设计上复用了data 让其传name进来又传结果出去,走到623行,传入name的任务已经完成,如果此处不进行清理那就是错误的传出去一个object对象,虽然外面的做了一个 if (data != name)
来过滤掉 找不到object的情况, 明显当前发现它并能正确的解决我们的需求,不如在退出函数前,将data修正NULL 避免外层做第二次判断
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.
首先,这没有解决 data == RT_NULL,data == (int)0x12344321 的场景。作为 iterator,对于 data 的实际 value 是不能做出任何假定的。
第二,从代码质量来说复用同一个变量做多件事情是不合理的。对于 reviewer 来说,难以理解含义也难以维护。这个修复只是把上个问题的直接现象解决了,却没有考虑到更深层次的问题,和核心矛盾。
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.
您说的 没错, rt_object_for_each的返回值并没有被很好的利用 能否考虑用它来做命中判定
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.
这个返回值最好就是只作为 rt_object_for_each
的返回值了,如果迭代器需要返回值,理应也是通过 data 传入一个自定义的对象,里面再包括一个 errno 的成员。不然这个东西越写越混淆,太容易误会了 😢
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.
我了解设计意图后,又进行了调整,感觉这样舒服点了,
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果进一步把 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