BIND 10 #275: refactoring CC session class for easier tests

BIND 10 Development do-not-reply at isc.org
Fri Jul 9 08:16:10 UTC 2010


#275: refactoring CC session class for easier tests
----------------------------------------+-----------------------------------
 Reporter:  jinmei                      |        Owner:  jinmei                     
     Type:  enhancement                 |       Status:  reviewing                  
 Priority:  major                       |    Milestone:  06. 4th Incremental Release
Component:  Inter-module communication  |   Resolution:                             
 Keywords:                              |    Sensitive:  0                          
----------------------------------------+-----------------------------------
Changes (by jelte):

  * owner:  jelte => jinmei


Comment:

 Replying to [comment:3 jinmei]:
 >
 > I've updated the description of the constructor.  But now that I've read
 the comment, I wonder it would be more natural if the caller must
 establish the session.  As a related matter, maybe we should explicitly
 detect and reject an attempt of doubly establishing a session? (I guess
 the underlying ASIO implementation detects it, but we should probably
 catch the invalid usage at a higher level).
 >

 Hmm, i think we either need to detect that establish is called twice, or
 we need to detect that it hasn't been called at all (and then we can
 choose to error or establish ourselves)



 > > For the sake of consistency, while not exactly essential, should we
 make *ccsession and *cs stack objects in auth's main.cc (and perhaps then
 rethink how we can do the same for auth_srv and io_service)?
 >
 > I'm not sure what this means.  Do you mean defining cc_session and cs in
 the anonymous namespace of auth/main.cc where auth_server and io_service
 are defined?  If so, I'd actually rather define the latter set more
 locally (because in general it's better to use a narrower scope for
 mutable variables).  But for now we need to define them in the namespace
 scope because they are referenced from free functions (my_config_handler
 and my_command_handler).  As a middle term plan we'll refactor this
 architecture with generalizing the ASIO link module, and then I think we
 can solve this in a cleaner way.  So I'd live with the inconsistency for
 now.
 >

 no, i mean not putting them on the heap
 {{{
     Session* cc_session = NULL;
     <snip>
         cc_session = new Session(io_service->get_io_service());
     <snip>
     delete cc_session;
 }}}
 but on the stack
 {{{
     Session cc_session(io_service->get_io_service());
 }}}

 And same for ModuleCCSession. Due to the current arch this isn't possible
 for auth_srv and io_service.

 But since we are in a main() function anyway this isn't really important,
 and we can probably defer this to a refactor later.

 If you want to so the check-session-for-established-already here, please
 do, for the rest i think this is ready for merge.

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


More information about the bind10-tickets mailing list