48 Star 231 Fork 92

OpenHarmony / communication_services_softbus_lite

代码RW:逻辑判断冗余

任务
已完成
whiten1ght  Opened this issue

该问题是怎么引起的?

逻辑判断冗余:

文件位置:communication_services_softbus_lite / authmanager / source / auth_conn.c:27

if ((buf == NULL) || (offset < 0) || (count <= 0) || (offset + count <= 0))

这里面最后一个判断(offset + count <= 0)不用判断了。前面已经保证了offset >= 0, count>0,那么offset + count > 0

重现步骤

代码review发现,,

报错信息

NA

+1 9
Attachments
8008844 whiten1ght 1599793375 2083969 panyox 1593271888 7349467 stephen jt 1584794603 2237213 xinulx 1593497122 total 7 participants

Comments (9)

robinyin 2020-09-11 10:48 member

我估计offset + count <= 0这句话的意图是在offset和count各自都大于零的前提下,应该是用于判断加法不要翻转。

理论上offset和count的各自被赋值构造一个比较大的正数(绕过前面的非负判断),然后翻转后访问到非法的内存上(前向溢出)

+1 6
robinyin 2020-09-11 10:53 member

多谢参与review,我们每个人都发出自己的星星之火,终究我们可以燎原

如果有其他问题,非常欢迎您提交PR和ISSUE
我们一起构造一个安全可靠和好用的操作系统 @whiten1ght

+1 5
8008844 whiten1ght 1599793375
whiten1ght 2020-09-11 10:59

ok

ShuningWan 2020-09-11 17:35

我估计offset + count <= 0这句话的意图是在offset和count各自都大于零的前提下,应该是用于判断加法不要翻转。
理论上offset和count的各自被赋值构造一个比较大的正数(绕过前面的非负判断),然后翻转后访问到非法的内存上(前向溢出)

@robinyin 前置调用代码
这部分调用代码应该不会出现因为很大的正数导致的相加为负,因为最大值看起来就是 offset + count == used + ( size - used ) == size :mag:

+1 3
2083969 panyox 1593271888
panyox 2020-09-12 01:53

为老兄点赞,翻了liteos下的评论,一群人在那里冷嘲热讽,没有一个人认真的去看代码,完全一副围观心态。如果是战争年代,那群人就是围观自己国人被日本鬼子随意屠杀而在一边幸灾乐祸,甚至还上去踢一脚。
华为加油,中华有为,国之幸矣!

+1 2
robinyin 2020-09-12 10:55 member

为老兄点赞,翻了liteos下的评论,一群人在那里冷嘲热讽,没有一个人认真的去看代码,完全一副围观心态。如果是战争年代,那群人就是围观自己国人被日本鬼子随意屠杀而在一边幸灾乐祸,甚至还上去踢一脚。
华为加油,中华有为,国之幸矣!

@panyox 咱们一起加油,聚集星星之火

7349467 stephen jt 1584794603
Stephen_jt 2020-09-12 11:01

为老兄点赞,翻了liteos下的评论,一群人在那里冷嘲热讽,没有一个人认真的去看代码,完全一副围观心态。如果是战争年代,那群人就是围观自己国人被日本鬼子随意屠杀而在一边幸灾乐祸,甚至还上去踢一脚。
华为加油,中华有为,国之幸矣!

@panyox 毕竟刚开始,围观的太多了,慢慢就会好起来了

2237213 xinulx 1593497122
wangshibao 2020-09-16 10:30

中国万岁!华为加油!

请叫我大哥 2020-09-17 16:25

take a look, all four opened issues are insignificant, what a shocking waste of time!我认为最重要的是有好的点子,或者扩展功能,或者是真实的bug,而不是形式上的合并某行两行代码,或者改个纠正下漏写的字母,这些都不是重点才是,并不影响看代码和功能运行,以及这个楼主没有去理解为何需要最后的判断就来提交commit,真的浪费时间,C语言里这么判断是有道理的,防止有符号数值相加溢出成负值,哎,质量不高啊。。。

Sign in to comment

Assignees
Labels
question
Projects
Milestones
Branches
Planed to start
Not set
Planed to end
Top level
Priority
1
https://git.oschina.net/openharmony/communication_services_softbus_lite.git
git@git.oschina.net:openharmony/communication_services_softbus_lite.git
openharmony
communication_services_softbus_lite
communication_services_softbus_lite

Search