Re: RFC: Use __attribute__ ((cleanup) in libvirt ?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux