NFD » ndn-cxx

  const uint8_t BUFFER[] = {0x80, 0x06, 0x81, 0x01, 0x01, 0x82, 0x01, 0x01};
  Block block(BUFFER, sizeof(BUFFER));
  [...]
  Buffer otherBuffer(BUFFER, sizeof(BUFFER));
  BOOST_CHECK_THROW(Block(block, otherBuffer.begin(), block.end()), std::invalid_argument);
  BOOST_CHECK_THROW(Block(block, block.begin(), otherBuffer.end()), std::invalid_argument);
  BOOST_CHECK_THROW(Block(block, otherBuffer.begin(), otherBuffer.end()), std::invalid_argument);

There are several problems here:

1. Depending on the memory allocator behavior, block and otherBuffer may be allocated contiguously one after the other, such that block.begin() == otherBuffer.end() or vice versa. If that's the case, the checks in Block() won't trigger an invalid_argument exception, but later readType()/readVarNumber() will throw a tlv::Error which is not caught by the BOOST_CHECK_THROW, thus causing a test failure.
2. The invocation of the Block constructor seen above is arguably invalid. The iterators being passed belong to different underlying buffers/containers. Given that comparing unrelated iterators is undefined behavior in C++, these invocations should not be considered valid code.
3. Taking a step back, what is the goal of these checks in the Block constructor? As pointed out above, nothing can fully prevent the caller from passing garbage arguments to the constructor, so those checks seem to have limited utility to me, while they introduce some (tiny) overhead. IMHO they should be BOOST_ASSERTs at most.