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

BIND 10 Development do-not-reply at isc.org
Fri Jul 9 18:28:08 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                          
Estimatedhours:                              |     Billable:  0                          
    Totalhours:                              |     Internal:  0                          
---------------------------------------------+------------------------------
Changes (by jinmei):

  * internal:  => 0
  * billable:  => 0


Comment:

 Replying to [comment:6 jelte]:
 > 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)
 >
 Let's think about it later.  I suspect a near future refactoring with
 a generalized ASIO link will affect the design on this, so it would
 make sense to revisit this issue at that point.  For the moment, I'll
 just leave a comment for the description of the ModuleCCSession
 constructor that the requirement is in flux.  I'll also make a
 separate ticket on this so that we (or trac) can remember the issue.
 >
 > no, i mean not putting them on the heap
 >
 Ah, okay.
 >
 > 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.
 >
 As you noted, with the current architecture we can't easily make this
 change, and that's the main reason why I used the new-delete pair.
 Also noted in the previous comment of mine, I expect we'll improve
 this part of the code in a cleaner way in a next step of refactoring.

 So, for now, let's live with the current implementation.
 >
 > If you want to so the check-session-for-established-already here, please
 do, for the rest i think this is ready for merge.
 >
 As noted above, I'd defer it to a separate task.  I'm going to merge
 the branch to trunk at this point because another important ticket
 (xfr/notify) depends on this ticket.

 Thanks for the review, btw:-)

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


More information about the bind10-tickets mailing list