BIND 10 #383: Generalize IOService constructor

[BIND 10 Development]

#383: Generalize IOService constructor
------------------------------+---------------------------------------------
      Reporter:  jelte        |        Owner:  jelte
          Type:  enhancement  |       Status:  new  
      Priority:  major        |    Milestone:       
     Component:  recurser     |   Resolution:       
      Keywords:               |    Sensitive:  0    
Estimatedhours:  0.0          |        Hours:  0    
      Billable:  1            |   Totalhours:  0    
      Internal:  0            |  
------------------------------+---------------------------------------------

Comment(by each):

 Replying to [comment:2 jelte]:
 > IOService still uses that Impl pattern (so other code does not have to
 (indirectly) include asio/io_service.hpp)

 Right.  If we de-pimpl IOService, we might as well get rid of it and just
 use asio::io_service.

 > and I now wonder whether DNSService (which is mostly the old IOService
 without the IOService-specific function (i.e. run(), stop() etc.) still
 needs to do the same. It doesn't for the sake op asio::io_service, but it
 currently uses those internal/*dns.h headers, which would then need to
 become public. Should we leave this as it is?

 I think we should keep it for that last reason--keeping tcpdns.h and
 udpdns.h hidden.

 Review notes:
 {{{
 +    /// These are currently very specific to the authoritative server
 +    /// implementation.
 }}}

 I hadn't realized this comment was still there; it's not correct anymore.

 Not sure why DNSService returns a native asio::io_service instead of an
 IOService, but it's fine with me either way.  (If it changes, though, it
 ought to be renamed getIOService(), both for clarity and for style
 compliance.  I assume IOService::get_io_service() was named the way it was
 because proper capitalization would be misleading about its return type;
 anyway, that's why I left it that way.)

 asiolink_unittest.cc:
 {{{
 +        if (dns_service_ != NULL) {
 +            delete dns_service_;
          }
 }}}

 Stephen mentioned in his review of #327 that checking for null around a
 delete isn't necessary; I forgot to fix that.

 main.cc:
 {{{
 +        io_service = new IOService();
 }}}

 io_service could be allocated on the stack now.  The only reason it wasn't
 before was that it needed to be instantiated down inside the if statement.

 recursor_unittest.cc:
 Changes are fine, but will also need to be made in auth_unittest.cc; I
 don't see that in the diff.

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