SDNV implementation for use in DTN protocols

SDNV implementation for use in DTN protocols

[Tom Henderson, 2014-05-21 01:36:10]

This looks good to me, once a few issues are addressed:

- improved class doxygen
- describe rationale for test case values
- we could use an example or two for src/network/examples.  How about:

1) enabling Python bindings and rewrite RFC 6256 appendix A but using ns-3's
methods (make it pass the same tests)?
2) create a simple C++ example Packet header structure that uses SDNV and show
how to add(serialize) and remove(deserialize) SDNV-encoded fields into packet
headers?


When we merge, I propose that we commit the sdnv.{cc,h} under Dizhi's
userstring, and the test code under Ruben's userstring, and whoever writes the
examples accordingly.

https://codereview.appspot.com/97540043/diff/1/src/network/test/sdnv-test-sui...
File src/network/test/sdnv-test-suite.cc (right):

https://codereview.appspot.com/97540043/diff/1/src/network/test/sdnv-test-sui...
src/network/test/sdnv-test-suite.cc:21: // Include a header file from your
module to test.
delete above comment

https://codereview.appspot.com/97540043/diff/1/src/network/test/sdnv-test-sui...
src/network/test/sdnv-test-suite.cc:23: #include "ns3/abort.h"
why is abort.h needed?

https://codereview.appspot.com/97540043/diff/1/src/network/test/sdnv-test-sui...
src/network/test/sdnv-test-suite.cc:65: //Prepare tests: testvector containing
unsigned integers with its corresponding encoded values
Please comment on where these test values come from?  In particular, what
boundary conditions or overflow conditions are you testing for?  Please describe
the testing strategy (rationale for these specific tests) here.

https://codereview.appspot.com/97540043/diff/1/src/network/utils/sdnv.h
File src/network/utils/sdnv.h (right):

https://codereview.appspot.com/97540043/diff/1/src/network/utils/sdnv.h#newco...
src/network/utils/sdnv.h:31: * \brief an implementation class of self-delimiting
numeric values based on RFC 6256
please add more detail here, along the lines of Natale's recent post (what, why,
how)?
http://mailman.isi.edu/pipermail/ns-developers/2014-May/011988.html

comment on whether it models (from RFC) "(2) the old style of
   SDNVs (both the SDNV-8 and SDNV-16) defined in an early stage of
   LTP's development [BRF04], or (3) the current SDNV format"

comment on maximum size supported

https://codereview.appspot.com/97540043/diff/1/src/network/utils/sdnv.h#newco...
src/network/utils/sdnv.h:80: * \brief is this the bolder of an encoded integer?
what is a 'bolder'?  Do you mean 'border'?

[Tommaso Pecorella, 2014-05-22 21:23:59]

I agree with Tom's comments. Plus, I'd remove the IsLast function.

https://codereview.appspot.com/97540043/diff/20001/src/network/utils/sdnv.h
File src/network/utils/sdnv.h (right):

https://codereview.appspot.com/97540043/diff/20001/src/network/utils/sdnv.h#n...
src/network/utils/sdnv.h:85: bool IsLast (uint8_t &val);
This function is used only once in the code.
Although it may be good for clarity, I'd suggest to remove it. Either it could
be declared private and inlined.