Opened 7 years ago

Closed 7 years ago

#2475 closed task (complete)

Split libdhcp++ and libdhcp_srv into separate folders.

Reported by: marcin Owned by: stephen
Priority: medium Milestone: Sprint-DHCP-20121129
Component: dhcp Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

This task involves moving libdhcp++ and libdhcp_srv into separate folders with separate makefiles. With the #2342 the dependency between libdhcp++ and libdhcp_srv has been introduced and this caused multi-threaded builds to fail because of the race condition between threads building both libraries (thread building libdhcp_srv expected libdhcp++ which has not been built yet). This race condition has been avoided by moving dependent classes (duid.{h, cc}) from libdhcp++ to libdhcp_srv and removing the dependency from Makefile.am. As this can't be treated as a permanent solution we need to separate both libraries. Another motivation to do this is the fact that there is no other folder in BIND10 that would hold two libraries.

Subtickets

Change History (9)

comment:1 Changed 7 years ago by marcin

  • Milestone changed from New Tasks to Sprint-DHCP-20121115

comment:2 Changed 7 years ago by stephen

  • Owner set to stephen
  • Status changed from new to assigned

comment:3 Changed 7 years ago by stephen

Note that solving this will also solve #2476

comment:4 Changed 7 years ago by stephen

  • Owner changed from stephen to marcin

Code has been split into dhcp and dhcpsrv directories, and "#include" directives altered appropriately.

The order of the include files now matches that recommended in the coding guidelines.

comment:5 Changed 7 years ago by marcin

  • Status changed from assigned to reviewing

comment:6 Changed 7 years ago by marcin

  • Owner changed from marcin to stephen

The copyright headers should be updated to 2012:

  • src/lib/dhcpsrv/tests/run_unittests.cc
  • src/lib/dhcpsrv/tests/addr_utilities_unittest/cc

src/lib/dhcpsrv/tests/addr_utilities_unittest.cc
Include of dhcpsrv/addr_utilities.h: Can it be moved before other headers?

src/bin/dhcp6/dhcp6_srv.h
Inlusion of iostream: Can iostream be moved to the end of includes list?

src/lib/dhcpsrv/subnet.h
Can boost includes be swapped with project includes?

src/lib/duid.cc
Can stdint.h be moved behind project includes?

... at this point I found that many other files in src/lib still have wrong order of includes. Is the reason for this that you aimed to fix includes order in only those files that you had to touch anyway when you were changing the path to dhcpsrv headers?

This change should have changelog entry associated with it as it touches library/common software that other software may depend on (if any exists apart from our server).

comment:7 Changed 7 years ago by stephen

  • Owner changed from stephen to marcin

The copyright headers should be updated to 2012:

Updated.

at this point I found that many other files in src/lib still have wrong order of includes. Is the reason for this that you aimed to fix includes order in only those files that you had to touch anyway when you were changing the path to dhcpsrv headers?

That's what I originally did, but in response to your comments, as well as the files you mention I thought that I may as well complete the job for lib/dhcp, lib/dhcpsrv, bin/dhcp4 and bin/dhcp6 (and the various "test" directories). The order of inclusion of header files now follows the coding guidelines, being in the order:

  1. config.h
  2. BIND 10 header files.
  3. Package header files. (e.g. Boost, MySQL, GTest)
  4. C++ header files.
  5. C header files.

I've also altered the #include "xxx.h" forms of the directive to #include <directory/xxx.h>, again following the coding guidelines. In addition, I checked that the "#include" sentinels conform to the format #ifndef FILENAME_H.

Suggested ChangeLog entry:

506.	[bug]		stephen
	Split the DHCP source files into two directories, each with its own Makefile.
        This properly solves the problem whereby a "make" operation with multiple
        threads could fail because of dependencies between the two libraries in the
        same directory.
	(Trac #2475, git xxx)

comment:8 Changed 7 years ago by marcin

  • Owner changed from marcin to stephen

Changes ok and can be merged.

Changelog
Maybe we could highlight that DHCP library files has been split, as DHCP source files may as well refer to server's source code. If you disagree for any reason, that's fine, you can commit as is. Please simply explain in this ticket why you disagree.

comment:9 Changed 7 years ago by stephen

  • Resolution set to complete
  • Status changed from reviewing to closed

Maybe we could highlight that DHCP library files has been split...

Done. First sentence starts "Split the DHCP library into ..."

Merged in commit 834fa9e8f5097c6fd06845620f68547a97da8ff8

Note: See TracTickets for help on using tickets.