Re: [Qemu-devel] [PATCH 0/8] Clean up #include "..." vs "<...>" and header guards

[Stefan Hajnoczi]

On Wed, Mar 20, 2019 at 02:54:22PM +0000, Daniel P. Berrangé wrote:
> On Wed, Mar 20, 2019 at 01:52:26PM +0000, Stefan Hajnoczi wrote:
> > On Fri, Mar 15, 2019 at 03:51:15PM +0100, Markus Armbruster wrote:
> > > * Our use of header guards is rather sloppy.  Sloppiness there can
> > >   lead to confusing compilation errors.  The rest of the series cleans
> > >   up existing header guards.  In particular, it normalizes guard
> > >   symbols to follow a common pattern, in the hope of making clashes
> > >   less likely.  It doesn't add new header guards.  We have more than
> > >   200 headers without a recognizable header guard.  A few of them are
> > >   for multiple inclusion, a few more don't need header guards because
> > >   they don't do anything but include, but the majority probably should
> > >   have one.  Left for another day.
> > 
> > Time for #pragma once?
> > https://en.wikipedia.org/wiki/Pragma_once
> 
> That seems like a reasonable idea to me. We already mandate CLang or
> GCC as only supported compilers, so there's no portability concerns.
> 
> 
> On the subject of non-standard C extensions, I was also wondering if
> QEMU should consider use of __attribute__((cleanup($func))) for doing
> automatically free'ing of memory when variables go out of scope. As
> well as reducing possibility of memory leaks, this simplifies code
> as it often allows for many "goto cleanup" jumps to be replaced with
> direct "return -1"s.  It isn't the most attractive syntax, but that
> downside doesn't outweigh the positives from my POV.
> 
> We recently started using it in libvirt for this reason.
> 
> Glib2 itself already uses this since 2.44 and exposes some macros for
> client code to use for this.
> 
> We bumped min glib to 2.40 a year go because Debian Jessie has 2.42
> and Ubuntu Trusty needs 2.40 which Peter used as a build merge test
> machine.
> 
> Our platform support policy will let us drop Jessie support when
> Buster is released fairly soon. If Peter no longer uses Trusty
> machines for buld, we'll soon be able to have a min glib that uses
> attribute(cleanup) as a standard dep.

That sounds good to me.  I've also seen __attribute__((cleanup($func)))
used more frequently in other projects and it makes the code both
simpler and safer.

Stefan

signature.asc
Description: PGP signature