BIND 10 #301: Review result for auth

[BIND 10 Development]

#301: Review result for auth
------------------------------+---------------------------------------------
      Reporter:  zhanglikun   |        Owner:  jinmei   
          Type:  enhancement  |       Status:  reviewing
      Priority:  major        |    Milestone:           
     Component:  b10-auth     |   Resolution:           
      Keywords:               |    Sensitive:  0        
Estimatedhours:  0.0          |        Hours:  0        
      Billable:  1            |   Totalhours:  3.0      
      Internal:  0            |  
------------------------------+---------------------------------------------
Changes (by stephen):

  * owner:  stephen => jinmei


Comment:

 >> src/bin/auth/auth_srv.cc
 >> With very similar names (!AuthSrv and !AuthSrvImpl), it might be
 clearer to put the
 >> declaration and implementation of !AuthSrvImpl into separate files.
 > I'm not sure if I understand this, but since !AuthSrvImpl is the
 implementation class
 > for !AuthSrv! using the pimpl idiom, we cannot separate !AuthSrvImpl
 into a different
 > .cc file. I noted it's for pimpl in auth_src.h.
 I was only suggesting that the contents of the auth_srv.cc file be split
 into multiple files - e.g. put the definition of !AuthSrvImpl in
 auth_srv_impl.h and the methods in auth_srv_impl.cc and do appropriate
 #includes.  The pimpl idiom - which in this case is that auth_srv.h is
 impervious to changes in the functionality of !AuthSrv(Impl) - is
 preserved.

 However, it's only a style suggestion and there is no need to make any
 changes.


 >> The !AuthSrv setXxxx and getXxxx functions are sufficiently trivial
 that consideration
 >> could be given to defining them inline in the header file.
 > It could, but for impl we cannot. In any case these are not expected to
 be performance
 > sensitive operations, so I think it's okay.
 Fair point.


 >> Check with Jerry Chen about recent (9 August 2010) changes to this file
 (in
 >> processAxfrQuery) concerning problems with AXFRs failing because of
 connection
 >> problems.
 > I don't understand what this comment means. Please clarify.
 I saw something authored by Jerry Chen that mentioned these problems, but
 I can't now find it in either my email records or the Jabber logs.  My
 fault, I should have copied the information here.  Ignore it - if it is
 still a problem, it will doubtless appear during testing.

 All other changes are OK.

 Please go ahead and merge.

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