BIND 10 #259: b10-auth and bind10 cleanup comments

BIND 10 Development do-not-reply at isc.org
Tue Jun 29 19:03:39 UTC 2010


#259: b10-auth and bind10 cleanup comments
--------------------------+-------------------------------------------------
 Reporter:  each          |       Owner:     
     Type:  defect        |      Status:  new
 Priority:  major         |   Milestone:     
Component:  Unclassified  |    Keywords:     
Sensitive:  0             |  
--------------------------+-------------------------------------------------
 While reviewing trac #167, Stephen made some comments about
 src/bin/auth/main.cc and src/bin/bind10 that were unrelated to the ticket
 at hand; I am moving those comments into this ticket to be addressed
 later.

 > Review of branches/trac167 revision 2268
 >
 > Files modified/added since the branch was created at revision 1879
 > (output of {{{svn diff -r 1879:HEAD --summarize}}}).
 >
 > M       src/bin/auth/main.cc
 > M       src/bin/bind10/tests/bind10_test.py
 > M       src/bin/bind10/bind10.py.in
 >
 >
 > '''src/bin/auth/main.cc'''
 > __General__
 > The documentation in this module is sparse.
 >
 > The opening brace of some functions is not on the end of the signature
 when the signature is on one line (BIND-10 C++ coding standards).
 >
 >
 > __check_axfr_query__
 > A C-style cast is used - the C++ style "xxx_cast<>()" is preferred.
 >
 > A "magic" number (0xFC80) is hard-coded in the function.
 >
 > The single-line "if" blocks should be surrounded by braces (BIND-10 C++
 coding standards).
 >
 >
 > __dispatch_axfr_query__
 > A C-style cast is used - the C++ style "xxx_cast<>()" is preferred
 >
 >
 > __class TCPClient__
 > No indication given should an error occur; instead the object silently
 disappears.
 >
 >
 > __class UDPServer__
 > Does not appear to be any logging should an error occur in the receive
 function, nor if a zero-length message is received.
 >
 > If B10_FROM_SOURCE is defined, the path "/src/bin/auth/auth.spec" is
 appended to it to file the specification file.  Just a bit unhappy that
 this path is buried deep in the code - it would be better either declared
 as a constant at the head of the file or placed into a separate header
 file.
 >
 >
 > '''src/bin/bind10/tests/bind10_test.py'''
 > __TestProcessInfo__
 > ''test_init''
 > Should check that pi.dev_null_stderr is also set to False on default
 construction.
 >
 > "env" is an argument to the !ProcessInfo constructor.  Why have the
 tests for the initialization of this member been commented out?
 >
 > __TestBob__
 > Only checks that the arguments have been stored correctly.  What tests
 are done that the other methods work?
 >
 >
 > '''src/bin/bind10/bind10.py.in'''
 > (Just a check of the changes since revision 1879)
 >
 > __Class !ProcessInfo__
 > Remark: As this class creates a process, to my untutored eyes
 !ProcessInfo seems a misleading name.

-- 
Ticket URL: <http://bind10.isc.org/ticket/259>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list