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