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