Re: __attribute__ ((cleanup) patch

[Pavel Simerda <psimerda redhat com>]

> > We have been too conservative to adopt the C99 standard and now we
> > are brave enough to go for non-standard compiler features.
> 
> We already use some C99 features; basic stuff like __func__ and
> snprintf().  I don't have a problem with using "-std=c99" to build
> stuff, but it seems like either GCC is already exposing those
> features
> with -std=gnu89.  The one thing I *don't* like in c99 is the mixed
> declarations and code; that clutters up the code horribly.

This is a matter of opinion. Creating a variable when I need it may be
sometimes much more readable. And, without that, you can't use variable
sized automatic arrays which are one of the things Colin's patch is
good for (although g_alloca works well here, too).

> > 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 is not a goal of NetworkManager to actively support other
> platforms.

We're not talking about active support here, but about potentially
actively blocking this. And I'm the last one to actively oppose
the BSD or *any other* community from working with us.

> If there was a nice, easy way to support *BSD (and with the platform
> stuff maybe there would be) then we'd entertain the patch, but we
> have enough things to work on at the moment than BSD support.

Give me an answer to the following questions.

1) Do we consider the Linux non-GCC community if *they* want to use
NetworkManager?

2) Do we consider the BSD community if *they* want to port NetworkManager?

3) Do we consider the possibility that someone else, non-Linux, non-BSD would
like to work join the NetworkManager effort, use it and help us with the development?

I think that regardless the actual technical aspects, by doing what we do here, we say:
“We are NetworkManager. We are Linux. We are GCC. Otherwise, fuck off.”

On the other hand, reverting, and starting again in a community-positive way, we would
be saying: “Yes, you are welcome to work with us and we are ready to listen to your
concerns and *then* decide.”

I'll be meeting with SUSE networking developers during the weekend. What should I tell
them? That it's not worth trying to work with us unless they are ready to see big changes
going under their hands?

> > > 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.
> 
> That doesn't work for allocated values passed back from other
> functions,
> which happens quite a bit.  But the libgsystem stuff does work for
> that
> AFAIK.  ie:

I'm sorry but I was reacting solely to the patch as there was no public
discussion. Plus I did some quick googling, but you shouldn't expect that.
And that's me, an active developer.

The community needs more than that to accomodate the changes. And if we do
rude things like that, why didn't we drop libnl1/2. This might have saved
more work. Yes, and other dependencies.

I believe that this is a matter of attitude. Either we prove to be assholes
who don't care, or we maintain a healthy open source project. It's not
possible to do both.

> char *foo;
> call_some_func (&foo);
> do_something
> g_free (foo);
> 
> cannot be made simpler with g_alloca().

True.

> > > /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.
> 
> I don't consider this an experiment at this point.

That surprises me.

> We're unlikely to convert to C++ any time soon, and anything that makes
> C easier to code, reduces programmer errors and memory leaks, and cleans up the
> existing code is a win, as long as it's not too complicated.  Given that this
> stuff is *less* complicated than the existing code, it's a win.  Yes,
> there is a bit of education required, but perhaps we could also work
> on making some things compile-time errors instead of runtime gotchas.

I'm not comfortable seeing these changes made by force (git push to master). What
I would expect, is to have a public discussion, with arguments for, arguments against,
and an educated decision afterwards.

> > > 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.
> 
> We use GCC.  We don't use obscure features of GCC.  We don't adhere
> blindly to C standards, we deviate where it makes sense.  Yes, this
> issue is up for discussion,

Thank you.

> but I also haven't heard anything that
> makes me think a revert is in order.

I guess to show that we know we screwed up is not good enough. Talking
about a change that has been merge is ranting. Talking about a change
that is being considered is a discussion.

I'm not going to pretend a discussion over something that has been already
decided.

Pavel

> Dan
> 
> > 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
> > _______________________________________________
> > networkmanager-list mailing list
> > networkmanager-list gnome org
> > https://mail.gnome.org/mailman/listinfo/networkmanager-list
> 
> 
>