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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>
> ---
> CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
> Â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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel