Fix the converters between the old and new BIO_read functions to handle end-of-file properly.#29290
Fix the converters between the old and new BIO_read functions to handle end-of-file properly.#29290igus68 wants to merge 1 commit intoopenssl:masterfrom
Conversation
| * This is essentially the same as BIO_read_ex() except that it allows | ||
| * 0 or a negative value to indicate failure (retryable or not) in the return. | ||
| * This is essentially the same as BIO_read_ex() except that it returns 0 in | ||
| * case of EOF and a negative value to indicate failure (retryable or not). |
There was a problem hiding this comment.
I really wonder if the original comment was not trying to say that 0 is retryable error and negative return is not. However it is really undocumented.
There was a problem hiding this comment.
Implementation of BIO_read() definitely relies on the return values of bio_read_intern() described in my version of this comment.
There was a problem hiding this comment.
It is a very poor description of an internal function to compare it to a public function which is implemented by it. Could we rewrite it properly saying what the internal function itself does?
There was a problem hiding this comment.
I've tried to rewrite it in the proper way.
| Functions set by BIO_meth_set_write_ex(), BIO_meth_set_write(), | ||
| BIO_meth_set_read_ex() and BIO_meth_set_read() must call BIO_set_flags() to | ||
| set the BIO_FLAGS_SHOULD_RETRY flag in relevant situations. | ||
|
|
|
This needs to be rebased after the reformat |
|
Rebased |
|
ping @openssl/committers for the second review |
|
There are some considerations in the comment #8208 (comment) that may cause a redesign of this solution, so I've converted this PR to a draft. |
|
@mattcaswell please re-review |
end-of-file state properly. Related to openssl/project#1745
|
CI claims that BIO_eof() should be mentioned in CHANGES.md as a new function. I'm not sure it is really needed in case of converting a macro into a function. |
|
ping @openssl/committers for second review |
|
This pull request is ready to merge |
end-of-file state properly. Related to openssl/project#1745 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> MergeDate: Thu Feb 12 08:34:31 2026 (Merged from #29290)
|
Merged to master as be42447. Thank you! |
end-of-file state properly. Related to openssl/project#1745 Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> MergeDate: Thu Feb 12 08:34:31 2026 (Merged from openssl#29290)
Fixes openssl/project#1745
Checklist