Re: [PATCH] tools/toollog: Drop XTL_NEW_LOGGER()

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2016-01-14 at 20:13 +0000, Andrew Cooper wrote:
> XTL_NEW_LOGGER() makes a number of unreasonable assumptions about the symbols
> visible in its scope,

It assumes that the function names to fill in the vtable and the type name
are related, that hardly seems totally "unreasonable". What else does it
assume that makes it unreasonable?

I think if you intend to remove something on this basis you need to be
specific about what you believe the short comings are.

>  and as such is only usable by its sole caller.

"not usable by every imaginable caller" is not the same as "usable by one
single possible caller", I think you are overstating the case here.

> 
> Remove it.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libs/toollog/include/xentoollog.h | 21 ---------------------
>  tools/libs/toollog/xtl_logger_stdio.c   | 30 ++++++++++++++++++---------
> ---
>  2 files changed, 18 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/libs/toollog/include/xentoollog.h
> b/tools/libs/toollog/include/xentoollog.h
> index 853e9c7..2b5bfcb 100644
> --- a/tools/libs/toollog/include/xentoollog.h
> +++ b/tools/libs/toollog/include/xentoollog.h
> @@ -112,25 +112,4 @@ void xtl_progress(struct xentoollog_logger *logger,
>  
>  const char *xtl_level_to_string(xentoollog_level); /* never fails */
>  
> -
> -#define XTL_NEW_LOGGER(LOGGER,buffer)
> ({                                \
> -    xentoollog_logger_##LOGGER
> *new_consumer;                           \
> -                                                                        
> \
> -    (buffer).vtable.vmessage =
> LOGGER##_vmessage;                       \
> -    (buffer).vtable.progress =
> LOGGER##_progress;                       \
> -    (buffer).vtable.destroy  =
> LOGGER##_destroy;                        \
> -                                                                        
> \
> -    new_consumer =
> malloc(sizeof(*new_consumer));                       \
> -    if (!new_consumer)
> {                                                \
> -        xtl_log((xentoollog_logger*)&buffer,                            
> \
> -                XTL_CRITICAL, errno,
> "xtl",                             \
> -                "failed to allocate memory for new message
> logger");    \
> -    } else
> {                                                            \
> -        *new_consumer =
> buffer;                                         \
> -    }                                                                   
> \
> -                                                                        
> \
> -    new_consumer;                                                       
> \
> -});
> -
> -
>  #endif /* XENTOOLLOG_H */
> diff --git a/tools/libs/toollog/xtl_logger_stdio.c
> b/tools/libs/toollog/xtl_logger_stdio.c
> index 0cd9206..8bce1a7 100644
> --- a/tools/libs/toollog/xtl_logger_stdio.c
> +++ b/tools/libs/toollog/xtl_logger_stdio.c
> @@ -165,28 +165,34 @@ void
> xtl_stdiostream_adjust_flags(xentoollog_logger_stdiostream *lg,
>  
>  xentoollog_logger_stdiostream *xtl_createlogger_stdiostream
>          (FILE *f, xentoollog_level min_level, unsigned flags) {
> -    xentoollog_logger_stdiostream newlogger;
>  
> -    newlogger.f = f;
> -    newlogger.min_level = min_level;
> -    newlogger.flags = flags;
> +    xentoollog_logger_stdiostream *nl =
> +        calloc(sizeof(xentoollog_logger_stdiostream), 1);
> +
> +    if (!nl)
> +        return NULL;
> +
> +    nl->vtable.vmessage = stdiostream_vmessage;
> +    nl->vtable.progress = stdiostream_progress;
> +    nl->vtable.destroy  = stdiostream_destroy;
> +
> +    nl->f = f;
> +    nl->min_level = min_level;
> +    nl->flags = flags;
>  
>      switch (flags & (XTL_STDIOSTREAM_PROGRESS_USE_CR |
>                       XTL_STDIOSTREAM_PROGRESS_NO_CR)) {
> -    case XTL_STDIOSTREAM_PROGRESS_USE_CR: newlogger.progress_use_cr = 1;
> break;
> -    case XTL_STDIOSTREAM_PROGRESS_NO_CR:  newlogger.progress_use_cr = 0;
> break;
> +    case XTL_STDIOSTREAM_PROGRESS_USE_CR: nl->progress_use_cr = 1;
> break;
> +    case XTL_STDIOSTREAM_PROGRESS_NO_CR:  nl->progress_use_cr = 0;
> break;
>      case 0:
> -        newlogger.progress_use_cr = isatty(fileno(newlogger.f)) > 0;
> +        nl->progress_use_cr = isatty(fileno(nl->f)) > 0;
>          break;
>      default:
>          errno = EINVAL;
>          return 0;
>      }
>  
> -    if (newlogger.flags & XTL_STDIOSTREAM_SHOW_DATE) tzset();
> -
> -    newlogger.progress_erase_len = 0;
> -    newlogger.progress_last_percent = 0;
> +    if (nl->flags & XTL_STDIOSTREAM_SHOW_DATE) tzset();
>  
> -    return XTL_NEW_LOGGER(stdiostream, newlogger);
> +    return nl;
>  }
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel