You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2020/09/24 20:06:32 UTC

[GitHub] [qpid-dispatch] jiridanek opened a new pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

jiridanek opened a new pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853


   My attempt to rescue code from the following commit in the following PR.
   The code has diverged since, and I had to edit manually.
   
   https://github.com/nicob87/qpid-dispatch/commit/4fc99cf08d033817c02bc83a6d74ddb8f87515af
   
   https://github.com/jiridanek/qpid-dispatch/compare/jd_nb_2020_09_24-mocking...nicob87:4fc99cf08d033817c02bc83a6d74ddb8f87515af
   
   https://github.com/apache/qpid-dispatch/pull/684


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek edited a comment on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek edited a comment on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-698563800


   There is multiple options how to do this. I've been writing a more complete list, but I only have an example for two.
   
   ## preprocessor magic
   
   See the PR
   
   ## elfspy
   
   https://github.com/mollismerx/elfspy
   
   The code below, plus need to add a library
   
   ```
   #include "qdr_doctest.h"
   #include "elfspy/SPY.h"
   #include "elfspy/Fake.h"
   
   int fake_vsnprintf(char * s, size_t n, const char * format, va_list arg) {
       return -1;
   }
   
   TEST_CASE("safe_snprintf_vsnprintf_failed") {
       const int   OUTPUT_SIZE = 128;
       const char *TEST_MESSAGE = "something";
       const int   LEN = strlen(TEST_MESSAGE);
       size_t len;
       char output[OUTPUT_SIZE];
   
       // weird elfspy boilerplate
       char* argv[2] = {(char*)"c_unittests", nullptr};
       spy::initialise(0, argv);
   
       // setup the fake, it will be unset when the variables go out of scope
       // it changes dynamic linking in the running process, to replace call to vsnprintf in glibc with the fake
       auto vsnprintf_ = SPY(&vsnprintf);
       auto vsnprintf_fake = spy::fake(vsnprintf_, &fake_vsnprintf);
       // alternative way for previous line:
       // auto vsnprintf_fake = spy::fake(vsnprintf_, [](auto ...) { return -1; });  // I can haz lambdaz !!!
   
       // run the test, simulating a failed vsnprintf
       output[0] = 'a';
       len = safe_snprintf(output, LEN+10, TEST_MESSAGE);
       CHECK(0 == len);
       CHECK('\0' == output[0]);
       CHECK("" == output);
   }
   ```
   
   ## more possibilities
   
   * use the `__wrap` feature in gnu linker
   * libraries similar to elfspy, such as mimick, https://github.com/Snaipe/Mimick
   * linking order, just define vsnprintf in the same translation unit as the test and things work out just right
   * LD_PRELOAD
   * ...
   
   ## general consideration
   
   In any case, static functions cannot be mocked in C, and also inlining, when compiler decides to optimize this way, destroys the ability to replace a function. That implies that this kind of tests can be either only run in a debug build, or it has to build a second, `-testing` version of the dispatch library, for this testing. Third option is not to link to the dispatch library and `#include` the needed .c files. That also works, if the surface area tested is small. It encourages creating many smaller ctest test targets, instead of one large one.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek edited a comment on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek edited a comment on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-698563800


   There is multiple options how to do this. I've been writing a more complete list, but I only have an example for two.
   
   ## preprocessor magic
   
   See the PR
   
   ## elfspy
   
   https://github.com/mollismerx/elfspy
   
   The code below, plus need to add a library
   
   ```c++
   #include "qdr_doctest.h"
   #include "elfspy/SPY.h"
   #include "elfspy/Fake.h"
   
   int fake_vsnprintf(char * s, size_t n, const char * format, va_list arg) {
       return -1;
   }
   
   TEST_CASE("safe_snprintf_vsnprintf_failed") {
       const int   OUTPUT_SIZE = 128;
       const char *TEST_MESSAGE = "something";
       const int   LEN = strlen(TEST_MESSAGE);
       size_t len;
       char output[OUTPUT_SIZE];
   
       // weird elfspy boilerplate
       char* argv[2] = {(char*)"c_unittests", nullptr};
       spy::initialise(0, argv);
   
       // setup the fake, it will be unset when the variables go out of scope
       // it changes dynamic linking in the running process, to replace call to vsnprintf in glibc with the fake
       auto vsnprintf_ = SPY(&vsnprintf);
       auto vsnprintf_fake = spy::fake(vsnprintf_, &fake_vsnprintf);
       // alternative way for previous line:
       // auto vsnprintf_fake = spy::fake(vsnprintf_, [](auto ...) { return -1; });  // I can haz lambdaz !!!
   
       // run the test, simulating a failed vsnprintf
       output[0] = 'a';
       len = safe_snprintf(output, LEN+10, TEST_MESSAGE);
       CHECK(0 == len);
       CHECK('\0' == output[0]);
       CHECK("" == output);
   }
   ```
   
   ## more possibilities
   
   * use the `__wrap` feature in gnu linker
   * libraries similar to elfspy, such as mimick, https://github.com/Snaipe/Mimick
   * linking order, just define vsnprintf in the same translation unit as the test and things work out just right
   * LD_PRELOAD
   * ...
   
   ## general consideration
   
   In any case, static functions cannot be mocked in C, and also inlining, when compiler decides to optimize this way, destroys the ability to replace a function. That implies that this kind of tests can be either only run in a debug build, or it has to build a second, `-testing` version of the dispatch library, for this testing. Third option is not to link to the dispatch library and `#include` the needed .c files. That also works, if the surface area tested is small. It encourages creating many smaller ctest test targets, instead of one large one.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek edited a comment on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek edited a comment on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-698563800


   There is multiple options how to do this. I've been writing a more complete list, but I only have an example for two.
   
   ## preprocessor magic
   
   See the PR
   
   ## elfspy
   
   https://github.com/mollismerx/elfspy
   
   The code below, plus need to add a library
   
   ```
   #include "qdr_doctest.h"
   #include "elfspy/SPY.h"
   #include "elfspy/Fake.h"
   
   TEST_CASE("safe_snprintf_vsnprintf_failed") {
       const int   OUTPUT_SIZE = 128;
       const char *TEST_MESSAGE = "something";
       const int   LEN = strlen(TEST_MESSAGE);
       size_t len;
       char output[OUTPUT_SIZE];
   
       // weird elfspy boilerplate
       char* argv[2] = {(char*)"c_unittests", nullptr};
       spy::initialise(0, argv);
   
       // setup the fake, it will be unset when the variables go out of scope
       // it changes dynamic linking in the running process, to replace call to vsnprintf in glibc with the fake
       auto vsnprintf_ = SPY(&vsnprintf);
       auto vsnprintf_fake = spy::fake(vsnprintf_, &fake_vsnprintf);
       // alternative way for previous line:
       // auto vsnprintf_fake = spy::fake(vsnprintf_, [](auto ...) { return -1; });  // I can haz lambdaz !!!
   
       // run the test, simulating a failed vsnprintf
       output[0] = 'a';
       len = safe_snprintf(output, LEN+10, TEST_MESSAGE);
       CHECK(0 == len);
       CHECK('\0' == output[0]);
       CHECK("" == output);
   }
   ```
   
   ## more possibilities
   
   * use the `__wrap` feature in gnu linker
   * libraries similar to elfspy, such as mimick, https://github.com/Snaipe/Mimick
   * linking order, just define vsnprintf in the same translation unit as the test and things work out just right
   * LD_PRELOAD
   * ...
   
   ## general consideration
   
   In any case, static functions cannot be mocked in C, and also inlining, when compiler decides to optimize this way, destroys the ability to replace a function. That implies that this kind of tests can be either only run in a debug build, or it has to build a second, `-testing` version of the dispatch library, for this testing. Third option is not to link to the dispatch library and `#include` the needed .c files. That also works, if the surface area tested is small. It encourages creating many smaller ctest test targets, instead of one large one.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek commented on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-701609625


   The decision is to pursue dynamic PLT/GOT modification approach, such as elfspy. It is self contained in the test source code, and has the least number of requirements on the compilation process. The disadvantage is that it becomes harder to ma the tests work multiplatform.
   
   There are various other libraries besides elfspy, I examined five so far, but each seems to have at least one significant limitation.
   
   I'll close this PR, since we chose not to use preprocessor solution prototyped here. I will put further updates on the Jira.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek closed pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek closed pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek commented on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-698563800


   There is multiple options how to do this. I've been writing a more complete list, but I only have an example for two.
   
   ## linking magic
   
   See the PR
   
   ## elfspy
   
   https://github.com/mollismerx/elfspy
   
   The code below, plus need to add a library
   
   ```
   #include "qdr_doctest.h"
   #include "elfspy/SPY.h"
   #include "elfspy/Fake.h"
   
   TEST_CASE("safe_snprintf_vsnprintf_failed") {
       const int   OUTPUT_SIZE = 128;
       const char *TEST_MESSAGE = "something";
       const int   LEN = strlen(TEST_MESSAGE);
       size_t len;
       char output[OUTPUT_SIZE];
   
       // weird elfspy boilerplate
       char* argv[2] = {(char*)"c_unittests", nullptr};
       spy::initialise(0, argv);
   
       // setup the fake, it will be unset when the variables go out of scope
       // it changes dynamic linking in the running process, to replace call to vsnprintf in glibc with the fake
       auto vsnprintf_ = SPY(&vsnprintf);
       auto vsnprintf_fake = spy::fake(vsnprintf_, &fake_vsnprintf);
       // alternative way for previous line:
       // auto vsnprintf_fake = spy::fake(vsnprintf_, [](auto ...) { return -1; });  // I can haz lambdaz !!!
   
       // run the test, simulating a failed vsnprintf
       output[0] = 'a';
       len = safe_snprintf(output, LEN+10, TEST_MESSAGE);
       CHECK(0 == len);
       CHECK('\0' == output[0]);
       CHECK("" == output);
   }
   ```
   
   ## more possibilities
   
   * use the `__wrap` feature in gnu linker
   * libraries similar to elfspy, such as mimick, https://github.com/Snaipe/Mimick
   * linking order, just define vsnprintf in the same translation unit as the test and things work out just right
   * LD_PRELOAD
   * ...
   
   ## general consideration
   
   In any case, static functions cannot be mocked in C, and also inlining, when compiler decides to optimize this way, destroys the ability to replace a function. That implies that this kind of tests can be either only run in a debug build, or it has to build a second, `-testing` version of the dispatch library, for this testing. Third option is not to link to the dispatch library and `#include` the needed .c files. That also works, if the surface area tested is small. It encourages creating many smaller ctest test targets, instead of one large one.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek commented on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-698563800


   There is multiple options how to do this. I've been writing a more complete list, but I only have an example for two.
   
   ## linking magic
   
   See the PR
   
   ## elfspy
   
   https://github.com/mollismerx/elfspy
   
   The code below, plus need to add a library
   
   ```
   #include "qdr_doctest.h"
   #include "elfspy/SPY.h"
   #include "elfspy/Fake.h"
   
   TEST_CASE("safe_snprintf_vsnprintf_failed") {
       const int   OUTPUT_SIZE = 128;
       const char *TEST_MESSAGE = "something";
       const int   LEN = strlen(TEST_MESSAGE);
       size_t len;
       char output[OUTPUT_SIZE];
   
       // weird elfspy boilerplate
       char* argv[2] = {(char*)"c_unittests", nullptr};
       spy::initialise(0, argv);
   
       // setup the fake, it will be unset when the variables go out of scope
       // it changes dynamic linking in the running process, to replace call to vsnprintf in glibc with the fake
       auto vsnprintf_ = SPY(&vsnprintf);
       auto vsnprintf_fake = spy::fake(vsnprintf_, &fake_vsnprintf);
       // alternative way for previous line:
       // auto vsnprintf_fake = spy::fake(vsnprintf_, [](auto ...) { return -1; });  // I can haz lambdaz !!!
   
       // run the test, simulating a failed vsnprintf
       output[0] = 'a';
       len = safe_snprintf(output, LEN+10, TEST_MESSAGE);
       CHECK(0 == len);
       CHECK('\0' == output[0]);
       CHECK("" == output);
   }
   ```
   
   ## more possibilities
   
   * use the `__wrap` feature in gnu linker
   * libraries similar to elfspy, such as mimick, https://github.com/Snaipe/Mimick
   * linking order, just define vsnprintf in the same translation unit as the test and things work out just right
   * LD_PRELOAD
   * ...
   
   ## general consideration
   
   In any case, static functions cannot be mocked in C, and also inlining, when compiler decides to optimize this way, destroys the ability to replace a function. That implies that this kind of tests can be either only run in a debug build, or it has to build a second, `-testing` version of the dispatch library, for this testing. Third option is not to link to the dispatch library and `#include` the needed .c files. That also works, if the surface area tested is small. It encourages creating many smaller ctest test targets, instead of one large one.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek commented on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek commented on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-698926372


   ## linking order solution (first found symbol wins)
   
   https://github.com/apache/qpid-dispatch/blob/b5deb03579a7dedd81a56f32baa3e5f4b5b57136/tests/timer_test.c#L37-L44
   
   (this is already being used in Dispatch in one test)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


[GitHub] [qpid-dispatch] jiridanek edited a comment on pull request #853: WIP: DISPATCH-1783 testing safe_snprintf function

Posted by GitBox <gi...@apache.org>.
jiridanek edited a comment on pull request #853:
URL: https://github.com/apache/qpid-dispatch/pull/853#issuecomment-698563800






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org