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