Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
当fd为0的时候,ret不会返回-1么?
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.
或者说fd没取到合法的值的时候,ret应该返回-1才对。还是说有可能ret和fd都是0?
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.
同意,如果有问题,也应该修改
BIO_get_fd
内部的逻辑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.
我看了下BIO_get_fd的实现,如果该BIO没有初始化,应该返回-1,其余情况都是返回正常的fd值(即fd和ret值相等)。如果不是这种情况,那可能BIO_get_fd的实现就有问题了
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.
BIO_get_fd内部实现没有问题。内部调用的sock_ctrl 返回值成功的话,ret = bio->num,bio->num 可能为0。
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.
如果要是这样,那么逻辑应该是:以
BIO_get_fd()
的返回值作为判断是否调用成功的依据,如果是-1
则直接失败,大于等于0
的都认为是成功:改一行即可
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.
改一行是不行的。ret=0有两种场景:
1.成功:调用的sock_ctrl(b->method->ctrl)返回值成功的话,ret = bio->num,bio->num 可能为0。
2.失败:BIO_get_fd失败也有可能返回0
验证BIO_get_fd宏得到fd=0的场景,可以通过静态编译链接到nginx,然后nginx全局配置daemon off; 单次请求必现。
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.
正常启动进程fd=0会被占用,但是不能排除fd=0的场景。像nginx以非守护进程的方式启动就是例子。
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.
失败不会反回0,失败只会返回-1。所以返回0的时候,fd必为0。在成功的情况下,ret的值和fd的值是相同的。失败的时候ret为-1
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.
失败返回0的情况只有在bio==NULL的场景,在SSL_connection_is_ntls函数调用时确实是不会出现的。
但是从openssl中使用 BIO_get_fd 这个宏的地方,一般都不会依赖于返回值,而是依赖于输出参数fd的值,fd>=0就认为是合法的。