[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/5] Add a metadata parameter to virLog{, V}Message



On Wed, Oct 17, 2012 at 08:17:14PM +0200, Miloslav Trmač wrote:
> ... and update all users.  No change in functionality, the parameter
> will be used later.
> 
> The metadata representation is as minimal as possible, but requires
> the caller to allocate an array on stack explicitly.
> 
> The alternative of using varargs in the virLogMessage() callers:
> * Would not allow the caller to optionally omit some metadata elements,
>   except by having two calls to virLogMessage.
> * Would not be as type-safe (e.g. using int vs. size_t), and the compiler
>   wouldn't be able to do type checking
> * Depending on parameter order:
>   a) virLogMessage(..., message format, message params...,
>                    metadata..., NULL)
>      can not be portably implemented (parse_printf_format() is a glibc
>      function)
>   b) virLogMessage(..., metadata..., NULL,
>                    message format, message params...)
>      would prevent usage of ATTRIBUTE_FMT_PRINTF and the associated
>      compiler checking.
> 
> Signed-off-by: Miloslav Trmač <mitr redhat com>
> ---
>  src/util/logging.c   |  6 +++++-
>  src/util/logging.h   | 24 ++++++++++++++++++------
>  src/util/viraudit.c  |  4 ++--
>  src/util/virterror.c |  2 +-
>  4 files changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/src/util/logging.c b/src/util/logging.c
> index 9a8bba1..71fb6bd 100644
> --- a/src/util/logging.c
> +++ b/src/util/logging.c
> @@ -729,6 +729,7 @@ virLogVersionString(const char **rawmsg,
>   * @filename: file where the message was emitted
>   * @linenr: line where the message was emitted
>   * @funcname: the function emitting the (debug) message
> + * @metadata: NULL or metadata array, terminated by an item with NULL key
>   * @fmt: the string format
>   * @...: the arguments
>   *
> @@ -741,13 +742,14 @@ virLogMessage(virLogSource source,
>                const char *filename,
>                int linenr,
>                const char *funcname,
> +              virLogMetadataPtr metadata,
>                const char *fmt, ...)
>  {
>      va_list ap;
>      va_start(ap, fmt);
>      virLogVMessage(source, priority,
>                     filename, linenr, funcname,
> -                   fmt, ap);
> +                   metadata, fmt, ap);
>      va_end(ap);
>  }
>  
> @@ -759,6 +761,7 @@ virLogMessage(virLogSource source,
>   * @filename: file where the message was emitted
>   * @linenr: line where the message was emitted
>   * @funcname: the function emitting the (debug) message
> + * @metadata: NULL or metadata array, terminated by an item with NULL key
>   * @fmt: the string format
>   * @vargs: format args
>   *
> @@ -771,6 +774,7 @@ virLogVMessage(virLogSource source,
>                 const char *filename,
>                 int linenr,
>                 const char *funcname,
> +               virLogMetadataPtr metadata ATTRIBUTE_UNUSED,
>                 const char *fmt,
>                 va_list vargs)
>  {
> diff --git a/src/util/logging.h b/src/util/logging.h
> index c67377f..024814f 100644
> --- a/src/util/logging.h
> +++ b/src/util/logging.h
> @@ -61,7 +61,7 @@ typedef enum {
>   */
>  # ifdef ENABLE_DEBUG
>  #  define VIR_DEBUG_INT(src, filename, linenr, funcname, ...)           \
> -    virLogMessage(src, VIR_LOG_DEBUG, filename, linenr, funcname, __VA_ARGS__)
> +    virLogMessage(src, VIR_LOG_DEBUG, filename, linenr, funcname, NULL, __VA_ARGS__)
>  # else
>  /**
>   * virLogEatParams:
> @@ -78,11 +78,11 @@ static inline void virLogEatParams(virLogSource unused, ...)
>  # endif /* !ENABLE_DEBUG */
>  
>  # define VIR_INFO_INT(src, filename, linenr, funcname, ...)             \
> -    virLogMessage(src, VIR_LOG_INFO, filename, linenr, funcname, __VA_ARGS__)
> +    virLogMessage(src, VIR_LOG_INFO, filename, linenr, funcname, NULL, __VA_ARGS__)
>  # define VIR_WARN_INT(src, filename, linenr, funcname, ...)             \
> -    virLogMessage(src, VIR_LOG_WARN, filename, linenr, funcname, __VA_ARGS__)
> +    virLogMessage(src, VIR_LOG_WARN, filename, linenr, funcname, NULL, __VA_ARGS__)
>  # define VIR_ERROR_INT(src, filename, linenr, funcname, ...)            \
> -    virLogMessage(src, VIR_LOG_ERROR, filename, linenr, funcname, __VA_ARGS__)
> +    virLogMessage(src, VIR_LOG_ERROR, filename, linenr, funcname, NULL, __VA_ARGS__)
>  
>  # define VIR_DEBUG(...)                                                 \
>      VIR_DEBUG_INT(VIR_LOG_FROM_FILE, __FILE__, __LINE__, __func__, __VA_ARGS__)
> @@ -93,6 +93,16 @@ static inline void virLogEatParams(virLogSource unused, ...)
>  # define VIR_ERROR(...)                                                 \
>      VIR_ERROR_INT(VIR_LOG_FROM_FILE, __FILE__, __LINE__, __func__, __VA_ARGS__)
>  
> +
> +struct _virLogMetadata {
> +    const char *key;
> +    const char *s;              /* String value, or NULL to use "i" */
> +    int i;
> +};
> +
> +typedef struct _virLogMetadata virLogMetadata;
> +typedef struct _virLogMetadata *virLogMetadataPtr;
> +
>  /**
>   * virLogOutputFunc:
>   * @src: the src for the message
> @@ -164,14 +174,16 @@ extern void virLogMessage(virLogSource src,
>                            const char *filename,
>                            int linenr,
>                            const char *funcname,
> -                          const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(6, 7);
> +                          virLogMetadataPtr metadata,
> +                          const char *fmt, ...) ATTRIBUTE_FMT_PRINTF(7, 8);
>  extern void virLogVMessage(virLogSource src,
>                             virLogPriority priority,
>                             const char *filename,
>                             int linenr,
>                             const char *funcname,
> +                           virLogMetadataPtr metadata,
>                             const char *fmt,
> -                           va_list vargs) ATTRIBUTE_FMT_PRINTF(6, 0);
> +                           va_list vargs) ATTRIBUTE_FMT_PRINTF(7, 0);
>  extern int virLogSetBufferSize(int size);
>  extern void virLogEmergencyDumpAll(int signum);
>  #endif
> diff --git a/src/util/viraudit.c b/src/util/viraudit.c
> index 61f09eb..9bdc5ed 100644
> --- a/src/util/viraudit.c
> +++ b/src/util/viraudit.c
> @@ -106,11 +106,11 @@ void virAuditSend(const char *filename,
>          if (success)
>              virLogMessage(VIR_LOG_FROM_AUDIT, VIR_LOG_INFO,
>                            filename, linenr, funcname,
> -                          "success=yes %s", str);
> +                          NULL, "success=yes %s", str);
>          else
>              virLogMessage(VIR_LOG_FROM_AUDIT, VIR_LOG_WARN,
>                            filename, linenr, funcname,
> -                          "success=no %s", str);
> +                          NULL, "success=no %s", str);
>      }
>  
>  #if HAVE_AUDIT
> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index 9572116..df8b6fd 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -679,7 +679,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED,
>      virLogMessage(virErrorLogPriorityFilter ? VIR_LOG_FROM_FILE : VIR_LOG_FROM_ERROR,
>                    priority,
>                    filename, linenr, funcname,
> -                  "%s", str);
> +                  NULL, "%s", str);
>  
>      errno = save_errno;
>  }

ACK

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]