BIND 10 #1966: logger formatter does too much in the constructor

[BIND 10 Development]

#1966: logger formatter does too much in the constructor
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |                       Status:  new
            Priority:  medium        |                    Milestone:  Next-
           Component:  logging       |  Sprint-Proposed
           Sensitive:  0             |                     Keywords:
         Sub-Project:  DNS           |              Defect Severity:  N/A
Estimated Difficulty:  0             |  Feature Depending on Ticket:
         Total Hours:  0             |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
 While working on #1964, I noticed the destructor of the log::Formater
 class had quite a lot of responsibility.  In the context of #1964, the
 check mode can throw an exception, which is obviously bad.  Even with
 forgetting it, the destructor calls Logger::output(), which internally
 calls log4cplus modules that we cannot directly control and cannot be
 really sure if it's really safe.

 Although I agree it's a neat trick to let the destructor finish the
 output, it seems to me quite fragile to rely on at such a sensitive
 place as a destructor.

 I propose an alternative approach, which should be much safer in this
 regard, and hopefully will not damage the convenience of the current
 notation too much:

 - introduce another method to `Formatter`, say, output().  It
   calls logger_->output().  In the check mode, it also performs the
   "excess placeholder" check, and throws an exception if it finds an
   error.  It records the fact it's called so that it cannot be called
   more than once (if broken, throw).
 - (not absolutely necessary but for convenience) also introduce
   another additional method, say, endarg().  It's similar to arg(),
   but it also calls the newly introduced output().
 - the `Formatter` constructor now only does simpler and safer cleanup,
   such as deleting the message (btw, it seems to be possible that the
   message can leak depending on how the formatter is constructed.  We
   should also fix that).  Also, in the check mode, it checks whether
   print() has been called (or an exception has been thrown due to the
   format check error), and if not, abort the program by assert()
   (note that we cannot throw here).
 - The caller is responsible for completing the logging statements with
   either endarg() or explicit print().  If broken, we'll lose that log
   message in the normal mode, and we should be able to detect that via
   an automated test that uses the check mode.

 A bonus of this approach is that we can now use exception for the
 "excess placeholder" check, which was changed to assert() in #1964
 as an urgent care regression fix.

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