BIND 10 #2272: Improve the perfdhcp command_options_helper code.

[BIND 10 Development]

#2272: Improve the perfdhcp command_options_helper code.
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  stephen
  marcin                             |                Status:  reviewing
                       Type:         |             Milestone:  Sprint-
  defect                             |  DHCP-20121004
                   Priority:         |            Resolution:
  medium                             |             Sensitive:  0
                  Component:         |           Sub-Project:  DHCP
  Unclassified                       |  Estimated Difficulty:  0
                   Keywords:         |           Total Hours:  0
            Defect Severity:  N/A    |
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by marcin):

 * owner:  marcin => stephen


Comment:

 Replying to [comment:7 stephen]:
 > Reviewed commit 3148cf2b018b222e263894b80cd7e92b20c86ceb
 >
 > Corrections to the code are OK.
 >
 > > That would work as well but my intention was to separate errors that
 come purely from invocation of CommandOptions::parse() from errors that
 come from helper methods, which are only to prepare the input to parse().
 > I wouldn't worry about that - "new" is used all over the place, both in
 tests and in the code, so the extra information as to what exception was
 thrown is far outweighed (IMHO) by the non-standard use of "new".  If
 memory is really so short, it would probably show up in other tests
 anyway.

 Ok. The code will now throw exception if new fails to allocate memory. The
 missing part is ASSERT_NO_THROW checks on call to process() in
 command_options_unittest.cc. I haven't added it here because it has been
 added with #1960 so once we merge both trac2272 and trac1960 to master it
 will be there.

 >
 > > I am not sure if I understand the comment
 > Your original comment seemed to state:
 > * on BSD/MacOS, optreset is available.  optreset must be set to 1.  In
 addition, optind must be set to 1.
 > * on Linux, there is no optreset.  optind must be set to 0.
 >
 > The final case, which is not mentioned is neither BSD, MacOS or Linux
 (e.g. Solaris).  Assuming that optind is set to 0 in these cases, the
 logic is:
 > {{{
 > #ifdef HAVE_OPTRESET
 > optind = 1;
 > optreset = 1;
 >
 > #else
 > optind = 0;
 >
 > #endif
 > }}}
 > i.e. there does not need to be a dependency on !__GLIBC!__.
 >
 >

 I checked that this will not work on Solaris 10. Solaris does not support
 optreset. Also setting optind=0 results in seg fault. The optind=0 is not
 a rule but it is the exception that exists for GLIBC only and is not
 portable.

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