Bug #3724
closed
Face: bad_function_call when InterestCallback/DataCallback/NackCallback is empty
100%
Description
Steps to reproduce (1):
- invoke
Face::setInterestFilterwithInterestCallbackset to nullptr - invoke
Face::processEvents - cause the face to receive an Interest that matches the filter
Steps to reproduce (2):
- invoke
Face::expressInterestwithDataCallbackset to nullptr - invoke
Face::processEvents - answer the Interest with a Data
Steps to reproduce (2):
- invoke
Face::expressInterestwithNackCallbackset to nullptr - invoke
Face::processEvents - answer the Interest with a Nack
Actual: throw std::bad_function_call
Expected: no such error
Updated by Junxiao Shi over 8 years ago
- Status changed from New to In Progress
- Assignee set to Junxiao Shi
- Estimated time set to 1.00 h
The fix is already part of https://gerrit.named-data.net/3025 patchset7, but I should make it explicit as a separate commit, and add test cases.
Updated by Junxiao Shi over 8 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
Updated by Junxiao Shi over 8 years ago
- Blocks Bug #3702: TopologyTester: does not process Nack added
Updated by Junxiao Shi over 8 years ago
- Blocks Bug #3703: AccessStrategy test suite: does not process Nack added
Updated by Junxiao Shi over 8 years ago
https://gerrit.named-data.net/3043 makes the test suite run faster.
Updated by Alex Afanasyev over 8 years ago
This is not an issue for me. Don't call expressInterest with nullptr callbacks, which don't makes much sense to me.
Even if such behavior is allowed, it should be implemented as a check inside expressInterest, not at the call time.
Updated by Davide Pesavento over 8 years ago
Alex Afanasyev wrote:
This is not an issue for me. Don't call expressInterest with nullptr callbacks, which don't makes much sense to me.
It's just for convenience... the check is cheap, and makes the API easier to use (it "just works").
Updated by Junxiao Shi over 8 years ago
This is not an issue for me. Don't call expressInterest with nullptr callbacks, which don't makes much sense to me.
It's permitted according to Doxygen https://github.com/named-data/ndn-cxx/blob/83872fd229b8d87e8e2aac25ba1dac3a69740bfd/src/detail/pending-interest.hpp#L77 but not correctly implemented.
Updated by Junxiao Shi over 8 years ago
The other direction to go is to forbid nullptr for all callbacks (except onTimeout on deprecated expressInterest), and assert this condition. There's no reason to require callbacks on Data and Nack, but not require callback on timeout.
Please let me know how to proceed.
Updated by Junxiao Shi over 8 years ago
Benchmark for https://gerrit.named-data.net/3043 patchset3:
Vagrant Ubuntu xenial 32-bit, debug build, static linking, execute time build/unit-tests -t TestFace three times
before: 53642ms, 55785ms, 46764ms
after: 4922ms, 3758ms, 3318ms
Updated by Junxiao Shi over 8 years ago
- Status changed from Code review to Closed