[Spice-devel] [PATCH v2 vdagent-win] vdlog: change log times to human readable date & time rhbz#672828

[Uri Lublin]

On 11/21/2011 09:50 AM, Arnon Gilboa wrote:
> -use RHEV log format
> -add log levels & macros
> -remove LOG_ENABLED ifdefs
> ---
>  common/vdlog.cpp |    4 ----
>  common/vdlog.h   |   54 +++++++++++++++++++++++++++++++++++++++---------------
>  2 files changed, 39 insertions(+), 19 deletions(-)
>
> diff --git a/common/vdlog.cpp b/common/vdlog.cpp
> index 1001de3..8ece384 100644
> --- a/common/vdlog.cpp
> +++ b/common/vdlog.cp
> +enum {
> +  LOG_DEBUG,
> +  LOG_INFO,
> +  LOG_WARN,
> +  LOG_ERROR,
> +  LOG_FATAL
> +};
>  
>  
> +#define PRINT_LINE(type, format, datetime, ms, ...)                                             \
> +    printf("%u::%s::%s,%.3d::%s::" format "\n", GetCurrentThreadId(), type, datetime, ms,       \
> +           __FUNCTION__, ## __VA_ARGS__);
> +
> +#define LOG(type, format, ...) if (type >= log_level) {                                         \
> +    VDLog* log = VDLog::get();                                                                  \
> +    const char *type_as_char[] = { "DEBUG", "INFO", "WARN", "ERROR", "FATAL" };                 \
> +    struct _timeb now;                                                                          \
> +    struct tm today;                                                                            \
> +    char datetime_str[20];                                                                      \
> +    _ftime_s(&now);                                                                             \
> +    localtime_s(&today, &now.time);                                                             \
> +    strftime(datetime_str, 20, "%Y-%m-%d %H:%M:%S", &today);                                    \
> +    if (log) {                                                                                  \
> +        log->PRINT_LINE(type_as_char[type], format, datetime_str, now.millitm, ## __VA_ARGS__); \
Note that "type" here      ^^^^^^^^^  can be larger than LOG_FATAL.
I think it's better to add a check for that.

> +    } else {                                                                                    \
> +        PRINT_LINE(type_as_char[type], format, datetime_str, now.millitm, ## __VA_ARGS__);      \
> +    }                                                                                           \
> +}
> +                                         
> +#define vd_printf(format, ...) LOG(LOG_INFO, format, ## __VA_ARGS__)
> +#define LOG_INFO(format, ...) LOG(LOG_INFO, format, ## __VA_ARGS__)

Nitpick/Style, I don't like using LOG_INFO (and others) for both the
logging operation
and the log level.

> +#define LOG_WARN(format, ...) LOG(LOG_WARN, format, ## __VA_ARGS__)
> +#define LOG_ERROR(format, ...) LOG(LOG_ERROR, format, ## __VA_ARGS__)

Do you want to add LOG_DEBUG()  or DBG() macros  ?

> +
> +#define ASSERT(x) _ASSERTE(x)
> +
>  void log_version();
>  
>  #endif