BIND 10 #1528: Interface detection on Linux refactoring

[BIND 10 Development]

#1528: Interface detection on Linux refactoring
-------------------------------------+-------------------------------------
                   Reporter:  tomek  |                 Owner:  stephen
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:  Sprint-
                   Priority:         |  DHCP-20120319
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  libdhcp                            |           Sub-Project:  DHCP
                   Keywords:         |  Estimated Difficulty:  0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by tomek):

 * owner:  tomek => stephen


Comment:

 Replying to [comment:6 stephen]:

 > ''IfaceMgr::setFlags''[[BR]]
 > Why are some members of !IfaceMgr public?  In particular, why have the
 flags_xxx_ members, which effectively duplicate the contents of flags_?
 Because bit meaning is different on different systems. I chose several
 flags that I believe will be useful for us, theses are not all flags,
 hence the original flags_ field. Also, keep note that not only definition,
 but also number of flags available on various systems is different.

 My idea was to hide those fields once we start supporting more than just
 Linux. Currently I do not have enough knowledge about different flavors of
 BSDs and Solaris to decide, which flags are common and which are not.

 > '''src/lib/dhcp/tests/iface_mgr_unittest.cc'''[[BR]]
 > When processing ifconfig output, you may want to consider use of
 isc::util::str::tokens, which splits output into separate tokens,
 regardless of the number of intervening spaces.  Processing tokens one at
 a time rather than searching through the string for the first space etc.
 may be easier. (This is a comment, no action required.)
 Added comment about isc::util::str::tokens.

 > parse_ifconfig: looking at the output of ifconfig on both FreeBSD
 (bikeshed) and Solaris (sol-10), it might be safer to regard the output
 for an interface as starting at the line where the first character is not
 whitespace.
 Aha! This code is Linux specific and it will stay that way. This is really
 a chicken and egg problem. If there was one simple, unified way of
 detecting interfaces across all platforms, we would have used it in the
 code in the first place. If there were two such methods available, we
 would have used the second one in tests. Sadly, there is not even one.

 Yes, parse_ifconfig has its limitations and it will stay that way. I was
 asked to implement tests for something that is not really fully testable.
 There are reports that parse_ifconfig breaks down even on Linux if there
 are interface aliases used. I'm sorry about that and offer my words of
 sympathy, but trying to make tests based on ifconfig working everywhere
 would be an exercise in futility. My recommendation hasn't changed: we
 must acknowledge that some code parts are better suited for testing than
 others.

 > Use spaces around binary operators (e.g. {{{line.find("inet")+5}}}).
 I hope I spotted and fixed them all.

 > '''src/lib/dhcp/tests/pkt4_unittest.cc'''[[BR]]
 > Test metaFields: other tests used boost::shared_ptr to store the newly-
 created Pkt4 object and to ensure destruction when the test exits, it
 would be logical for this to do so. (As an aside, use of shared_ptr in
 this context is an unnecessary overhead - if the pointer is not going to
 be shared, scoped_ptr would be better.)
 No, not at all. Consistency in tests is a fallacy. There is a value in
 trying to use tested code in different ways. In fact, as far as testing
 goes, the more variety the better. One may assume that the object should
 be destroyed in the same way, regardless if it is explicitly deleted using
 delete or destroyed by shared_ptr destructor. But again, tests should
 verify that rather than base on assumptions.

 All improvements are checked in and awaiting next round of review.

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