BIND 10 #1528: Interface detection on Linux refactoring

BIND 10 Development do-not-reply at isc.org
Tue May 8 12:51:39 UTC 2012


#1528: Interface detection on Linux refactoring
-------------------------------------+-------------------------------------
                   Reporter:  tomek  |                 Owner:  stephen
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:  Sprint-
                   Priority:         |  DHCP-20120514
  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:9 stephen]:
 > '''src/lib/dhcp/iface_mgr_linux.cc'''
 > I still had concern over the comments, so I've made some changes: only
 to the comments, although I've added/deleted some spaces.  The changes
 have been pushed: please review.
 Thanks. Changes looks ok.
 >
 >
 > >...BOOST_STATIC_ASSERT(sizeof(nlmsghdr) == offsetof(req,generic) ); But
 it does not work. Compiler complains about a comma not being allowed in
 macro. I think gcc is confused and thinks that two parameters are passed
 to BOOST_STATIC_ASSERT that takes only one.
 > I suggest using a plain "assert()" instead.  The run-time overhead is
 negligible but it is a useful additional check.
 I managed to make it work with BOOST_STATIC_ASSERT. It seems that it
 required name of a type, not a name of an instance of that type.

 > > And what benefit that would give us? I would rather work on something
 useful instead.
 > The additional overhead of making it an explicit class is not much, and
 it does make it easier to test and re-use in a C++ context should the need
 arise.
 Netlink is now a class, so I assume that comment point is closed.
 >
 > '''src/lib/dhcp/iface_mgr_linux.cc''' - additional points
 > open_socket() - cast should be done using C++ constructs.
 Done.

 > parse_rattr(): the check "<= table.size() - 1" could more easily be
 written as "< table.size()" (and saves the need to perform the subtraction
 before the test).
 Done.

 I believe that addresses all comments raised. Unless there are new
 comments, I think the code is ready for merge.

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


More information about the bind10-tickets mailing list