From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH RFC tools 1/6] tools: Refactor "xentoollog" into its own library Date: Thu, 11 Jun 2015 12:20:47 +0100 Message-ID: <55796F0F.7040406@citrix.com> References: <1433936188.30003.60.camel@citrix.com> <1433936205-21539-1-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1433936205-21539-1-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , xen-devel@lists.xen.org Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, roger.pau@citrix.com List-Id: xen-devel@lists.xenproject.org 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