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

Add missing BIO_should_read and BIO_should_write functions #5410

Closed
wants to merge 1 commit into from

Conversation

darktohka
Copy link
Contributor

@darktohka darktohka commented Jul 27, 2022

Description

Currently, BIO_should_retry exists as a component of the OpenSSL compatibility layer. However, there is no way to check the should_read/should_write flag of a BIO object - the BIO_should_read and BIO_should_write functions from OpenSSL are not implemented.

I've implemented the two missing functions as wolfSSL_BIO_should_read and wolfSSL_BIO_should_write by letting wolfSSL check the flags of the BIO, then created the compatibility definitions for BIO_should_read and BIO_should_write in the OpenSSL compatibility header.

Testing

I've created two unit tests, test_wolfSSL_BIO_should_read and test_wolfSSL_BIO_should_write. They each test the respective function before creating the connection and when a read or write is expected to happen.

I've ran the unit_test program after creating these unit tests, and all unit tests completed successfully.

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@wolfSSL-Bot
Copy link

Can one of the wolfSSL admins verify this patch?

@embhorn
Copy link
Member

embhorn commented Aug 1, 2022

Approved as contributor.

@embhorn
Copy link
Member

embhorn commented Aug 1, 2022

Okay to test

@dgarske
Copy link
Contributor

dgarske commented Aug 2, 2022

retest this please

@dgarske
Copy link
Contributor

dgarske commented Aug 3, 2022

@darktohka :

Testing: ./configure --enable-all
Running: make check
make[2]: warning: -j5 forced in submake: resetting jobserver mode.
tests/api.c:40013:10: error: unused variable 'ret' [-Werror,-Wunused-variable]
    int  ret;
         ^
tests/api.c:40051:14: error: variable 'bio' is uninitialized when used here [-Werror,-Wuninitialized]
    BIO_read(bio, reply, sizeof(reply));
             ^~~
tests/api.c:40014:13: note: initialize the variable 'bio' to silence this warning
    BIO* bio;
            ^
             = NULL
tests/api.c:40048:26: error: variable 'ssl' is uninitialized when used here [-Werror,-Wuninitialized]
    wolfSSL_SSLSetIORecv(ssl, EmbedReceive);
                         ^~~
tests/api.c:40009:21: note: initialize the variable 'ssl' to silence this warning
    WOLFSSL*     ssl;
                    ^
                     = NULL
tests/api.c:40084:10: error: unused variable 'ret' [-Werror,-Wunused-variable]
    int  ret;
         ^
tests/api.c:40121:14: error: variable 'bio' is uninitialized when used here [-Werror,-Wuninitialized]
    BIO_read(bio, reply, sizeof(reply));
             ^~~
tests/api.c:40085:13: note: initialize the variable 'bio' to silence this warning
    BIO* bio;
            ^
             = NULL
tests/api.c:40118:26: error: variable 'ssl' is uninitialized when used here [-Werror,-Wuninitialized]
    wolfSSL_SSLSetIORecv(ssl, EmbedReceive);
                         ^~~
tests/api.c:40080:21: note: initialize the variable 'ssl' to silence this warning
    WOLFSSL*     ssl;
                    ^
                     = NULL
6 errors generated.

Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the compiler errors in api.c.

@dgarske dgarske assigned darktohka and unassigned dgarske and embhorn Aug 3, 2022
@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

@JacobBarthelmeh
Copy link
Contributor

BIO_should_read and BIO_should_write was added here #6235

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants