> On Mon, Mar 31, 2008 at 06:37:13AM +0000, Jozef Misutka wrote:
>> 2Michal,
>> it would be good to use the logger class which is being used for API
>> logging - it is a cleaner solution for debugging and enables changing
>> output destination at runtime.
> Please name the advantages.
> Changing output destination is possible also for current implementation
> (with some minor changes - see bellow).
> I understand that you like having everything in objects and that it is
> nicer to have Formater for one job (formatting messages - also printing
> in your implementation) and Logger which handles higher level logic
> (where to log, some general prefix (maybe time information).
> But, current implementation of Formater doesn't bring more functionality
> than _printDbg macro which has significantly smaller code. Could you
> provide reasonable example where Formater class would do better (with
> current interface of Formater)?
> Logger class even complicates logging a bit, to enable preprocessor
> information providing to Formatter (splitting to init and print stages
> is necessary).
> I would expect that Logger will handle debug levels, but it seems not
> doing so.

1) object design vs. assembler ;)
1.5) reusability
2) kernel, gui, api will have different loggers which can be used to save these messages in different files etc...
3) formatter can be easily changed (formatter is not about new funcionality)
4) Logger is for logging purposes, does not need to be for debugging purposes and therefore it is really not suitable to put debugging level into it
(but it is a good point to change those functions to virtual to enable inheritance ..)
4.5) with inheritance: e.g. logger can have cyclic buffer

Debugging is functional so you are not forced to use it. APIlogger will use it.

>> example usage is the actual API logger implementation, for our logging
>> purposes the default Logger class should be enough.
>> debug::ApiLogger::logger.start_log ("", __FILE__, __FUNCTION__, __LINE__); \
>> debug::ApiLogger::logger << msg; \
>> debug::ApiLogger::logger.end_log();
> Is it really necessary to expand one line of message dumping into
> initialization and trailer for *each* message? We have performance
> problems with original implementation if all debug messages are enabled.
> This will be even more performance demanding.
>> #define APIDbg1(p1) _APIDbg((p1));
>> #define APIDbg2(p1,p2) _APIDbg((p1) << ", " << (p2));
>> #define APIDbg3(p1,p2,p3) _APIDbg((p1) << ", " << (p2) << ", " << (p3));
> What are these variants good for (just examples)?

every API function (at least my objects) will have this as the first line where p* will be input parameters).
you can create testcases easier with these logging messages.

>> this way, we can have 3 loggers, KERNEL, GUI and API.
> Summary:
> * I am trying to find some advantages, but except that you have turned
> core _printDbg functionality into 2 objects (60+ lines of code instead
> of 6 lines).
> * In the end you also need to use macros (__FILE__ ...). Btw. your macro
> definition of _APIDbg has exactly same flaw like the current one which
> I have fixed in the "[RFC] printDbg macros cleanup" patch.
> * I agree that we can create separate logger for each kernel, gui and
> API (means utils?) so they have different output, but is this what we do
> want?
> I don't mind if current
> #define kernelPrintDbg(dbgLevel, msg) printDbg("KERNEL", (dbgLevel), msg)
> is changed to
> #define kernelPrintDbg(dbgLevel, msg) _printDbg((prefix),(dbgLevel), debug::Logger::Kernel,msg);
> where debug::Logger::Kernel would be something that implements <<
> operator and maybe provide some higher level logic, like runtime
> redirecting, current debug level (instead of global variable).
> But your current implementation seems to me like overkill (it doesn't
> bring new functionality, extensions are not so easy IMO and it brings a
> lot of new code). I would get rid of Formater and use Logger as a simple
> stream with debugLevel inside.
>> as debug levels are your work, you can change the API debug level as
>> you wish.
> --
> Michal Hocko

Test your Star IQ Play now!