[Spice-devel] [vdagent-win PATCH 1/3] Make VDLog::printf static

[Christophe Fergeau]

On Wed, Jul 26, 2017 at 11:03:40AM +0100, Frediano Ziglio wrote:
> VDLog::printf is called only using a VDLog pointer.
> Being this class a singleton there could only one instance of
> the class so the pointer must be the single instance.
> This simplify LOG macros not having to choose between printf and
> VDLog::printf.

Maybe something like this instead:
"Since this class is a singleton, there can only be one instance of it,
so LOG() does not need really need to deal with it, VDLog::printf can do
that instead."


> The manual GNU __attribute__ is used as the format to use is
> not the standard __printf__ format used in SPICE_GNUC_PRINTF
> but you need to use the gnu_printf format.

I'd give an example or 2 of formats that require gnu_printf.

Looks good otherwise, 

Acked-by: Christophe Fergeau <cfergeau at redhat.com>

Christophe

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  common/vdlog.cpp |  5 +++--
>  common/vdlog.h   | 12 +++++-------
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/common/vdlog.cpp b/common/vdlog.cpp
> index b9ae93f..8af6dcc 100644
> --- a/common/vdlog.cpp
> +++ b/common/vdlog.cpp
> @@ -70,12 +70,13 @@ VDLog* VDLog::get(TCHAR* path)
>  
>  void VDLog::printf(const char* format, ...)
>  {
> +    FILE *fh = _log ? _log->_handle : stdout;
>      va_list args;
>  
>      va_start(args, format);
> -    vfprintf(_handle, format, args);
> +    vfprintf(fh, format, args);
>      va_end(args);
> -    fflush(_handle);
> +    fflush(fh);
>  }
>  
>  void log_version()
> diff --git a/common/vdlog.h b/common/vdlog.h
> index e645409..b1be391 100644
> --- a/common/vdlog.h
> +++ b/common/vdlog.h
> @@ -31,7 +31,10 @@ class VDLog {
>  public:
>      ~VDLog();
>      static VDLog* get(TCHAR* path = NULL);
> -    void printf(const char* format, ...);
> +#ifdef __GNUC__
> +    __attribute__((__format__ (gnu_printf, 1, 2)))
> +#endif
> +    static void printf(const char* format, ...);
>  
>  private:
>      VDLog(FILE* handle);
> @@ -61,7 +64,6 @@ static const VDLogLevel log_level = LOG_INFO;
>  
>  #define LOG(type, format, ...) do {                                     \
>      if (type >= log_level && type <= LOG_FATAL) {                       \
> -        VDLog* log = VDLog::get();                                      \
>          const char *type_as_char[] = { "DEBUG", "INFO", "WARN", "ERROR", "FATAL" }; \
>          struct _timeb now;                                              \
>          struct tm today;                                                \
> @@ -69,11 +71,7 @@ static const VDLogLevel log_level = LOG_INFO;
>          _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__); \
> -        } else {                                                        \
> -            PRINT_LINE(type_as_char[type], format, datetime_str, now.millitm, ## __VA_ARGS__); \
> -        }                                                               \
> +        VDLog::PRINT_LINE(type_as_char[type], format, datetime_str, now.millitm, ## __VA_ARGS__); \
>      }                                                                   \
>  } while(0)
>  
> -- 
> 2.13.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170726/ec22255b/attachment.sig>