[PATCH] ctdb-server: Remove one-line goto

[Andrew Bartlett]

On Fri, 2018-03-16 at 10:32 -0700, Jeremy Allison via samba-technical
wrote:
> 
> 
> Please bear with us, we (mostly:-) are really nice
> people and we'd love to help you help us - with the
> ultimate goal of adding you to the Team so you can
> be a nice person yourself :-) :-).

I'm mostly nice too :-).  I think they other point that was missed
above is that the Samba Team is, for better or worse, conservative.  

So, that means that there should be a point to changes, and so far
unless code is just totally unreadable, we don't normally change it
just for style.  Excuses have included:
 - loss of history (in git blame)
 - risk of code flow changes (prove this really doesn't change
anything)
 - (if it was so unreadable), are you sure you understood what it did
in the first place?
 - is there a test?

This isn't ideal, and this has a been a problem, but if you don't
realise it is part of our culture you can stumble on things
unexpectedly. 

It is also applied un-evenly.  Volker does a lot of clean-up patches
for example. 

In general, it is better to have a little more motivation.  Volker
often cites number of .text bytes saved when variables are initialised.
 I wonder if it ever really makes a difference, but it is generally
clearer code anyway and so it helps get things over the line.  

Andreas recently got a massive set of patches though that make us
compile with a stricter compiler option.

On the flip side, take real care around talloc.  talloc is as you have
been learning, really subtle at points, and has some extra features and
safety compared with malloc()/free() that Samba code uses
idiomatically, but which you can miss if you just read the API.  

Finally, I would say take it a bit more easy on the borderline patches.
 Things like "[PATCH] ctdb-server: Add goto tag avoiding code
duplication" as a case in point.  Both approaches are equally ugly, and
equally common in Samba, and when you have 20 patch threads on the go
it becomes easier for folks to throw up their hands and just ignore it
than dive in and look into it.

I hope this helps,

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba