BIND 10 #438: C++ Logging
BIND 10 Development
do-not-reply at isc.org
Wed Jan 19 14:56:32 UTC 2011
#438: C++ Logging
-------------------------------------+-------------------------------------
Reporter: stephen | Owner: stephen
Type: | Status: reviewing
enhancement | Milestone: R-Team-
Priority: major | Sprint-20110125
Component: logging | Resolution:
Keywords: | Sensitive: 0
Estimated Number of Hours: 5.0 | Add Hours to Ticket: 0
Billable?: 1 | Total Hours: 0
Internal?: 0 |
-------------------------------------+-------------------------------------
Changes (by jelte):
* owner: jelte => stephen
Comment:
General:
Is the range of debug levels (0-99) based on anything specific? I shall
have a hard time remembering more than, say, 5 levels :)
In a few files, i noticed a space after the ! operator in conditionals, is
this a style thing i wasn't aware of (we don't do that anywhere else)
Message.h:
is this file used or needed anywhere or is this leftover code?
(if so i have some comments, but i suspect the latter)
filename.*:
we should add some tests for corner cases in the split function
why is #ifdef WIN32 around normalize only in the header and not in the
normalize calls in expandwith and useas?
logger.h:
a bit of whitespace in the doxygen code would make it somewhat more
readable
ps. so now we have unfree'd memory on exit? once that problem gets fixed
in log4cxx we should consider an (optional) way to remove this behaviour,
if we want to run memory checkers on our code
there are two getters for warning enabled; getWarnEnabled() and
getWarningEnabled()
btw, should we provide some hints as to when to use which level?
Perhaps add an extra non-doxygen comment before the LoggerPtr* member in
the Logger class that this is a pointer to a Ptr because of the
static/dynamic delete problem mentioned earlier?
(btw, if we want to hide log4cxx completely we should not expose that
LoggerPtr in logger.h)
logger.cpp:
According to our coding guidelines, we should have a newline after the
return value of a function definition
strutil.cc:
just thought of something, perhaps you can also to the if win32 here (and
have the call simply be a nop on non-win systems)
No further comments on the rest of the code, that looks fine.
--
Ticket URL: <https://bind10.isc.org/ticket/438#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list