Re: [Xen-devel] [PATCH RFC tools 1/6] tools: Refactor "xentoollog" into its own library

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/06/15 12:36, Ian Campbell wrote:
> +
> +#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;                                                       \
> +});

This macro should be ditched.

It is a gnu-ism which shouldn't be present in the public library header,
violates several principles of least supprise, and can literally only be
used by its sole user in xtl_logger_stdio.c because of its internal
expectations of xentoollog_logger_stdiostream.  (Its sole user could do
the above in a cleaner manner anyway.)

As part of the tidyup, we should choose a particular C standard (89,
probably) and ensure that the API/ABI complies with `gcc -std=c$VER
-pedantic`.  This will help to provide a consistent API on other
platforms (I seem to recall an effort to port libvchan to windows.)

As another thought, it would also be a good time to sort out a
consistent coding style, although that doesn't necessarily need to be
folded into the split-out patch.  The current source is very mixed when
it comes to coding style.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel