Skip to content

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