[Spice-devel] [PATCH spice-common v3 2/2] log: Force format in log macro to be a string

[Frediano Ziglio]

> 
> On Tue, Jul 04, 2017 at 10:18:52AM +0100, Frediano Ziglio wrote:
> > Make sure format is a string and not a pointer.
> > This prevents usages like:
> > 
> >   spice_debug(NULL);
> > 
> > This also prevents computed format strings too but on the other
> > hand allows the compiler to check argument types.
> 
> This is vague and misleading, the compiler should already be able to
> check that we have a format string with the right arguments. This patch
> just adds a little bit on top of that, which is ensuring the format
> string is not NULL.
> 
> Christophe
> 

How can the compiler know if 

  spice_debug(my_string, 1, 2, 3);

is correct or not? The compiler must know the value of "my_string".
In the case of computed format strings the compiler cannot check types.
Currently we don't use computed strings but just constants or NULL,
with this patch code only allows constant strings.

> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> > Changes since v1:
> > - extent commit message rationale.
> > ---
> >  common/log.h | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/common/log.h b/common/log.h
> > index 9f5fcbb..06d48d2 100644
> > --- a/common/log.h
> > +++ b/common/log.h
> > @@ -62,23 +62,23 @@ void spice_log(GLogLevelFlags log_level,
> >  } G_STMT_END
> >  
> >  #define spice_info(format, ...) G_STMT_START {                         \
> > -    spice_log(G_LOG_LEVEL_INFO, SPICE_STRLOC, __FUNCTION__, format, ##
> > __VA_ARGS__); \
> > +    spice_log(G_LOG_LEVEL_INFO, SPICE_STRLOC, __FUNCTION__, "" format, ##
> > __VA_ARGS__); \
> >  } G_STMT_END
> >  
> >  #define spice_debug(format, ...) G_STMT_START {                         \
> > -    spice_log(G_LOG_LEVEL_DEBUG, SPICE_STRLOC, __FUNCTION__, format, ##
> > __VA_ARGS__); \
> > +    spice_log(G_LOG_LEVEL_DEBUG, SPICE_STRLOC, __FUNCTION__, "" format, ##
> > __VA_ARGS__); \
> >  } G_STMT_END
> >  
> >  #define spice_warning(format, ...) G_STMT_START {                       \
> > -    spice_log(G_LOG_LEVEL_WARNING, SPICE_STRLOC, __FUNCTION__, format, ##
> > __VA_ARGS__); \
> > +    spice_log(G_LOG_LEVEL_WARNING, SPICE_STRLOC, __FUNCTION__, "" format,
> > ## __VA_ARGS__); \
> >  } G_STMT_END
> >  
> >  #define spice_critical(format, ...) G_STMT_START {
> >  \
> > -    spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, format, ##
> > __VA_ARGS__); \
> > +    spice_log(G_LOG_LEVEL_CRITICAL, SPICE_STRLOC, __FUNCTION__, "" format,
> > ## __VA_ARGS__); \
> >  } G_STMT_END
> >  
> >  #define spice_error(format, ...) G_STMT_START {                         \
> > -    spice_log(G_LOG_LEVEL_ERROR, SPICE_STRLOC, __FUNCTION__, format, ##
> > __VA_ARGS__); \
> > +    spice_log(G_LOG_LEVEL_ERROR, SPICE_STRLOC, __FUNCTION__, "" format, ##
> > __VA_ARGS__); \
> >  } G_STMT_END
> >  
> >  #define spice_warn_if_fail(x) G_STMT_START {            \

Frediano