BIND 10 #2236: Add a --enable-debug configure flag

[BIND 10 Development]

#2236: Add a --enable-debug configure flag
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  vorner
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  low    |  Sprint-20121023
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  3
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => vorner


Comment:

 Hi Michal

 Replying to [comment:8 vorner]:
 > Hello
 >
 > The most important comment I have is, the most expensive debug thing to
 be disabled is this:
 > {{{#!c++
 > // TODO: Distinguish if debug mode is enabled in compilation.
 > // If so, it should be PTHREAD_MUTEX_NORMAL or NULL
 > result = pthread_mutexattr_settype(&attributes,
 PTHREAD_MUTEX_ERRORCHECK);
 > }}}
 >
 > About the locked multiple test, we probably want to disable it
 completely. Because most of it is ifdefed-out anyway and the only part
 left is the one that doesn't work with normal (fast) mutexes.
 >

 Updated. :)

 > The thing with config.h ‒ for one, it is not local header, it should
 definitely not be included with quotes, but angle brackets.

 We have to decide upon an include style that we can use in the entire
 tree, and add it to our `CodingGuidelines` wiki page. I'm following some
 convention when I write such includes.

 In my understanding, the choice of include style has nothing to do with
 whether the header is local or not.

 * `<>` replaces what's inside the delimiters by something that is
 implementation dependent.. it could be a file, or something else
 * `""` replaces what's inside the delimiters by a file (where it is found
 is implementation dependent.. usually -I, system directories, etc.)

 I spoke to Jinmei about it, and he said that we can defer it to when we do
 a cleanup of the entire tree based on what style we choose.

 > For another, while sync.h is mostly private header, I think we should
 distribute it (for one, because when we split the build into multiple
 packages, we need to refer to installed version). So I think we should
 ifdef inside the .cc file and not provide the methods in non-debug build
 (so they would cause linker error). And add a note about it to the
 documentation.

 I've moved the `config.h` to the .cc file now.

 >
 > Then, few minor things:
 >
 > There is some test or something left in the auth server:
 > {{{
 > auth_srv.o: In function
 `AuthSrv::swapDataSrcClientLists(boost::shared_ptr<std::map<isc::dns::RRClass,
 boost::shared_ptr<isc::datasrc::ConfigurableClientList>,
 std::less<isc::dns::RRClass>, std::allocator<std::pair<isc::dns::RRClass
 const, boost::shared_ptr<isc::datasrc::ConfigurableClientList> > > > >)':
 >
 /home/vorner/work/bind10/bind10-devel-20120817/_build/src/bin/auth/../../../../src/bin/auth/auth_srv.cc:939:
 undefined reference to `isc::util::thread::Mutex::locked() const'
 > auth_srv.o: In function
 `AuthSrvImpl::getDataSrcClientList(isc::dns::RRClass const&)':
 >
 /home/vorner/work/bind10/bind10-devel-20120817/_build/src/bin/auth/../../../../src/bin/auth/auth_srv.cc:278:
 undefined reference to `isc::util::thread::Mutex::locked() const'
 >
 /home/vorner/work/bind10/bind10-devel-20120817/_build/src/bin/auth/../../../../src/bin/auth/auth_srv.cc:278:
 undefined reference to `isc::util::thread::Mutex::locked() const'
 > collect2: ld returned 1 exit status
 > make[7]: *** [b10-auth] Error 1
 > make[7]: Leaving directory
 `/home/vorner/work/bind10/bind10-devel-20120817/_build/src/bin/auth'
 > }}}

 Ouch.. fixed.

 >
 > This comment may be removed or revised with the non-debug mode already
 existing.
 > {{{
 > // This assertion would fail only in non-debugging mode, in which case
 > // this method wouldn't be called either, so we simply assert the
 > // condition.
 > }}}
 >

 Removed.

 > The „enable debugging“ in configure, I think it should be made clear
 this is
 > just some expensive debugging, so that people don't fear they'd get no
 > debugging support at all when not enabling it and they don't enable it
 for
 > production code, because it would be slow.

 `configure.ac` has been updated for it.

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