[libvirt] RFC: Use __attribute__ ((cleanup) in libvirt ?

[Martin Kletzander]

On Tue, Jan 10, 2017 at 02:50:40PM +0100, Erik Skultety wrote:
>On Tue, Jan 10, 2017 at 02:17:00PM +0100, Martin Kletzander wrote:
>> On Tue, Jan 10, 2017 at 10:00:31AM +0000, Daniel P. Berrange wrote:
>> > On Tue, Jan 10, 2017 at 10:48:47AM +0100, Martin Kletzander wrote:
>> > > On Tue, Jan 10, 2017 at 02:58:21AM -0500, Laine Stump wrote:
>> > > > On 01/09/2017 08:09 PM, Richard W.M. Jones wrote:
>> > > > > On Mon, Jan 09, 2017 at 04:58:49PM +0000, Daniel P. Berrange wrote:
>> > > > > > For those who don't already know, GCC and CLang both implement a C language
>> > > > > > extension that enables automatic free'ing of resources when variables go
>> > > > > > out of scope. This is done by annotating the variable with the "cleanup"
>> > > > > > attribute, pointing to a function the compiler will wire up a call to when
>> > > > > > unwinding the stack. Since the annotation points to an arbitrary user
>> > > > > > defined function, you're not limited to simple free() like semantics. The
>> > > > > > cleanup function could unlock a mutex, or decrement a reference count, etc
>> > > > > >
>> > > > > > This annotation is used extensively by systemd, and libguestfs, amongst
>> > > > > > other projects. This obviously doesn't bring full garbage collection to
>> > > > > > C, but it does enable the code to be simplified. By removing the need to
>> > > > > > put in many free() (or equiv) calls to cleanup state, the "interesting"
>> > > > > > logic in the code stands out more, not being obscured by cleanup calls
>> > > > > > and goto jumps.
>> > > > > >
>> > > > > > I'm wondering what people think of making use of this in libvirt ?
>> > > > > >
>> > > > > > To my mind the only real reason to *not* use it, would be to maintain
>> > > > > > code portability to non-GCC/non-CLang compilers. OS-X, *BSD and *Linux
>> > > > > > all use GCC or CLang or both, so its a non-issue there. So the only place
>> > > > > > this could cause pain is people building libvirt on Win32, who are using
>> > > > > > the Microsoft compilers instead og GCC.
>> > > > > >
>> > >
>> > > Only reason I see for not using it is the "temporary" mess it will
>> > > cause.  Yes, we can change to that incrementally, but it will take some
>> > > time and effort and it will never be all of the code that uses it.
>> > > Don't get me wrong, I would love using more builtin compiler features
>> > > and shortening the code here and there.  I'm just worried this
>> > > particular one might be more disrupting than useful.  Most of us are
>> > > pretty used to the code flow we already have and there's nothing you
>> > > can't achieve without the cleanup attribute.
>> > >
>> > > And yes, I used quotation marks around the word temporary intentionally.
>> >
>> > Yes, that's why I thought of it as something that would make for a GSoc
>> > project - have someone do a full conversion of particular areas of code.
>> > eg convert all of util/ or convert the domain XML parser, etc. Basically
>> > if we did it, I think we'd want to have entire files converted at once.
>> > Only converting individual methods ad-hoc would be quite messy.
>> >
>>
>> Yes, I know, but that still doesn't mean all will be converted,
>> unfortunately.
>>
>
>Why not? I mean, the GSoC project doesn't need to be for just 1 student, if
>we're granted the slots you could pick multiple students who would work on it in
>parallel. Also, there are always means how we could keep track of it, an idea

You need:

  1) To be accepted as an organization
  2) Slots
  3) Students that are interested in rewriting cleanup all over the
     code.

You cannot depend on any of these.

>that first crossed my mind without giving any more thinking to it is that you
>can track the modules still waiting to be switched to the new convention within
>the GSoC section of our page. The 'virsh auto-completion' project has been in
>the 'ongoing' state for at least 2 years so I personally don't see an issue
>here, since it's a bigger task.
>

It's also been mentioned by some developers as "really messy" and it's
not something that changes how people read code in the whole codebase.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170110/1c40ba41/attachment-0001.sig>