BIND 10 #1265: DHCP benchmarking utility: send/receive DHCP packets

[BIND 10 Development]

#1265: DHCP benchmarking utility: send/receive DHCP packets
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  johnd
  stephen                            |                Status:  reviewing
                       Type:  task   |             Milestone:  Sprint-
                   Priority:  major  |  DHCP-20111123
                  Component:         |            Resolution:
  perfdhcp                           |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DHCP
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by stephen):

 * owner:  stephen => johnd


Comment:

 Reviewed commit b12941d14b2b30fedb0543c33c551b780ed6340b

 > I made PROGNAME a global again, since it's used in more than one module.
 It could just be {{{static const char* PROGNAME = "perfdhcp";}}} in the
 header - it would save the {{{extern}}} declaration and the chasing
 through the modules to find out where it is declared.  The odd few bytes
 wasted by multiple declarations are neither here nor there.

 '''tests/tools/perfdhcp/misc.h'''[[BR]]
 The name of the file is a bit generic, but let's see what else goes in
 there before changing it.

 reporterr() needs a function header.

 '''tests/tools/perfdhcp/netio.c'''[[BR]]
 > If you put the main dhcp* functions after the static helper functions in
 the file, you don't need to declare the latter.
 Any reason not to do this?

 socketSetup(): True that local_addr can only be null if the address family
 is IPV6, this is something that could perhaps be checked by an assertion
 (or a test that aborts with an error message).  The function is only
 called once and it would minimise the possibility of programming errors.

 addrPortToNames(): What is the "TODO" about - what is gai_strerror()?

 netShutdown(): This shuts down the connection started by dhcpSetup();
 dhcpShutdown() would be a better name for it.


 '''tests/tools/perfdhcp/tests/sendreceive_unittest.c'''[[BR]]
 The code in the V4 and V6 test is identical apart from the message and the
 command line.  The common code could be extracted into a separate
 function.  To identify which test produces the errors, use the
 SCOPED_TRACE macro, with the message being used for the test as the
 argument, e.g.
 {{{
 static void common_test(const char* argv[], int argvsize, const char*
 message) {
     SCOPED_TRACE(message);
            :
 }

 TEST(SendReceiveTest, v4) {
     const char* argv [] = {"perfdhcp", "127.0.0.1"};
     common_test(argv, (sizeof(argv) / sizeof(char*)), "This is the V4
 test");
 }

 TEST(SendReceiveTest, v6) {
     const char* argv [] = {"perfdhcp", "-6", "::1"};
     common_test(argv, (sizeof(argv) / sizeof(char*)), "This is the V6
 test");
 }

 }}}
 (Don't forget to replace {{{sizeof(message)}}} with {{{strlen(message)}}}
 in the code in common_test.)

 numOctets should be num_octets (variable naming).

-- 
Ticket URL: <http://bind10.isc.org/ticket/1265#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development