-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[kernel]将ipc中的上下文检查级别提高 #5576
[kernel]将ipc中的上下文检查级别提高 #5576
Conversation
这个放在下个版本,对api所处环境的检查确实很重要 |
@mysterywolf 好的,感谢回复。 到时候如需修改PR欢迎留言。 |
https://club.rt-thread.org/ask/article/3204.html 各类函数的用法,重点在于告诉用户怎么用,而不是让使用者毫无头绪的去乱试,每次遇到 assert 才恍然大悟,哦,这个函数不能这么用。这种想法是错的。 assert 是在开发阶段,或者说,是对入门者的一种快速暴力提示;对于一个熟练使用者来讲还依靠 assert 来定位用法错误,会对产品引入隐患的。 @mysterywolf 就像前不久在微信群里提到的那样,检测宏最好放到函数开头,醒目的地方,做“警示”“提示”作用。让使用者方便,快速,第一件需要了解到的事儿就是知道这个函数的使用环境。 这个 pr ,除了增加了代码大小,并不能引导使用者使用上的更多注意。 |
|
感谢回复。 我赞同您的这个观点,assert 过于暴力,不应该在这里使用。或许可以参考 linux kernel,强制打印一段提示:
而这个观点我认为有待商榷。 不应该说 README 里面写了,就把责任推给使用者。 受欢迎的产品常常是为用户着想,方便用户使用; 用户用着越方便,产品也就越受欢迎。 |
没有。 是我自己在 |
src/ipc.c
Outdated
@@ -515,7 +515,7 @@ rt_err_t rt_sem_take(rt_sem_t sem, rt_int32_t time) | |||
else | |||
{ | |||
/* current context checking */ | |||
RT_DEBUG_IN_THREAD_CONTEXT; | |||
RT_CHECK_IN_THREAD_CONTEXT; |
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_DEBUG是可以被关闭了,这样产品稳定之后就可以关闭这里的检查了。感觉后面的方向可以是让RT_DEBUG更细化,这里有单独的开关,最后关闭就好了。
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_DEBUG是可以被关闭了,这样产品稳定之后就可以关闭这里的检查了。感觉后面的方向可以是让RT_DEBUG更细化,这里有单独的开关,最后关闭就好了。
感谢回复。 我这个PR的主要想法是: 这里的上下文检查应该强制开启,不能关闭。 因为IPC通信的上下文“必须”是线程上下文,否则有可能会导致严重的后果,比如系统崩溃、死锁等,一定是错误的用法。
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.
是的,在产品开发阶段这个是要开着的,但是如果产品已经开发完毕,稳定之后,这里应该是可以关闭的。这样运行起来更快,产品固件的大小也更小。
拉取/合并请求描述:(PR description)
[
使用信号量、互斥锁等ipc通信机制时,要求上下文“必须”是线程上下文,否则会导致
严重的异常(死锁、hardfault等)。
上下文检查以宏定义的方式声明:
RT_DEBUG_IN_THREAD_CONTEXT
,此前一直位于rtdebug.h
文件中,并根据
RT_DEBUG
宏定义来控制开关。作为比较关键的检查,应该提高其重要级别,以防止有人关闭 RT_DEBUG 时,也自动关闭了
上下文检查。
参考:
FreeRTOS/queue.c :1542行
https://github.com/FreeRTOS/FreeRTOS-Kernel/blob/dca4f80a6be40f77848749ea18efd92b9d8ef66a/queue.c
Linux Kernel /mutex.c: 280行
https://github.com/torvalds/linux/blob/master/kernel/locking/mutex.c
Signed-off-by: Mingrui Ren [email protected]
]
以下的内容不应该在提交PR时的message修改,修改下述message,PR会被直接关闭。请在提交PR后,浏览器查看PR并对以下检查项逐项check,没问题后逐条在页面上打钩。
The following content must not be changed in the submitted PR message. Otherwise, the PR will be closed immediately. After submitted PR, please use a web browser to visit PR, and check items one by one, and ticked them if no problem.
当前拉取/合并请求的状态 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