Skip to content
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] 将rt_thread结构体改为显式继承rt_object #5500

Closed
wants to merge 2 commits into from
Closed

[kernel] 将rt_thread结构体改为显式继承rt_object #5500

wants to merge 2 commits into from

Conversation

mysterywolf
Copy link
Member

@mysterywolf mysterywolf commented Jan 9, 2022

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

[

struct rt_thread 结构体采用隐式继承rt_object,这样会带来非常大的风险。如果别人没有意识到二者之间的关系,调整或删除了rt_thread的结构体前几个成员变量的位置。就完蛋了。

]

以下的内容不应该在提交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):

  • 本拉取/合并请求是一个草稿版本 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

@mysterywolf mysterywolf changed the title [kernel] 将rt_thread结构体改为显式集成rt_object [kernel] 将rt_thread结构体改为显式继承rt_object Jan 9, 2022
@enkiller
Copy link
Contributor

这个改动比较猛啊,担心外部软件报或应用代码会受到较大的冲击

我建议添加一个 API ,获取当前线程的名字,这样子不用强制访问结构体获取信息

@mysterywolf
Copy link
Member Author

没关系 这个是4.1.0版本,可以有较大的变动,这个问题非常的危险,前两天海靖还在问我flag成员没啥用可否删除掉。

@mysterywolf mysterywolf added the urgent 🏃 urgent label Jan 12, 2022
@chenyingchun0312
Copy link
Contributor

chenyingchun0312 commented Jan 13, 2022

我觉得满老师一定是对代码美感有极致的追求,曾几何时,我在看rt_thread这个结构体的时候,看到了这里隐式的继承方式,但是其他的都是显示继承,就觉得很不舒服,我就在想,难道有特殊考虑?想破脑袋也没想出好在哪,这个修改+1,哈哈

@mysterywolf mysterywolf added the +1 Agree +1 label Jan 14, 2022
@BernardXiong
Copy link
Member

如果是这样,建议可以改得更猛烈些,其他代码都不能访问thread内部的结构体,全部都处理好(例如可以是宏,函数,inline函数等的方式)。可以尝试看看效果(例如新起分支,新的PR)

@mysterywolf mysterywolf added proposal proposal for future version and removed urgent 🏃 urgent labels Jan 14, 2022
@mysterywolf mysterywolf added urgent 🏃 urgent and removed proposal proposal for future version urgent 🏃 urgent labels Jan 16, 2022
@mysterywolf mysterywolf deleted the object branch January 16, 2022 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
+1 Agree +1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants