Re: __attribute__ ((cleanup) patch

[Pavel Simerda <psimerda redhat com>]

> From: "Colin Walters" <walters verbum org>
> Hi,

Hi, thanks for a quick reply.

> https://bugzilla.gnome.org/show_bug.cgi?id=685440
> 
> has a patch which just landed, but I wanted to give wider discussion
> to this, because it's a very important infrastructural change.
> 
> First, one thing that came up is a concern about a GCC hard
> dependency.

Yes.

> My understanding is that LLVM implements this too.

We have been too conservative to adopt the C99 standard and now we
are brave enough to go for non-standard compiler features.

> For compilers which implement C++, this is easy to support.

I don't deny this, but this is not how the question stands, currently.

> And since NetworkManager is highly tied to Linux

NetworkManager is tied to Linux, now, but in my opinion not as highly
as it might seem at the first glance. Dan might disagree.

> it's not like we have to care about MSVC.

Nobody's talking about MSVC here.

> Now, I plan to do a followup patch pretty soon which would let us
> get rid of a *lot* of g_free()

You can get rid of g_free's on small strings for free with a pretty
standard way through g_alloca, for example. For that, this is overkill.

> /g_object_unref() calls.

For this, I would need some more detailed information. This is just not enough
for merging someone's experiments that don't even have any documentation. I admit
it is tempting to use a real important project for one's experiments but it might
not be the right direction for the project.

> conservative estimate, around 1500 lines.
> I've been using libgsystem cleanup extensively in several projects,
> and it's been a massive win.

Then why it wasn't good enough for Glib but is good enough for NetworkManager? Why
I don't see a link to elaborate explanation and documentation? Success stories. More
information?

> Other examples of projects using this in C are systemd and upstart.
> 
> The downside is that this is kind of a one-way door.  Once done, it'd
> be immensely tedious to try to go back and add them again.

That's exactly the problem I have with such quick inclusions without a proper discussion.

> However, the benefit to the code is huge,

I'm not yet even convinced about this because of total lack of documentation to be
found right away. And I'm not convinced that switching to a non-standard programming
language is the price to pay.

> and even better - trust me, it makes writing C fun again =)

You mean writing a language incompatible with the C standards :). Changing
the programming language for the whole project in a forward-only compatible
manner is a big decision and I'm not really happy with the way it has been
done.

So, I haven't changed my mind. I propose the following path:

1) Revert

2) Document including the reasons why this cannot be added to Glib but should be
added to NetworkManager

3) Discuss

4) Decide

I don't think this should be a one-week process, so I personally insist on starting
with the step (1). I would like to discourage this practice for future decision
making regarding such huge one-way changes.

Cheers,

Pavel