[Swan] initial thoughts on uncrustifying libreswan

[Philippe Vouters]

Dear Hugh,

I do backup your work toward simplifying much Libreswan. The code has 
become so uselessly complex that Libreswan has become very hard to 
maintain, to use and to support. Not only counting upon the fact that 
the more uselessly complex it is, the more it is subject to instability, 
bugs and regressions. All the logging related useless macros are no 
exception.

One important item in any software either open source or proprietary 
which makes me mad are regressions. They cause everyone to loose lots of 
time and energy. Too frequent regressions are one item which disgusted 
me in participating to Libreswan. As I wrote Paul and write anyone, 
making things complex is simple; making things simple is complex.To my 
personal long lasting experience, this is what makes the real difference 
between a true professional and an amateur.

Philippe Vouters (Fontainebleau/France)
URL: http://vouters.dyndns.org/
SIP: sip:Vouters at sip.linphone.org

On 05/23/2013 09:43 AM, D. Hugh Redelmeier wrote:
> Something like this is a really good idea.  The current source is a ragged
> mess.
>
> A radical suggestion is that all code checked into libreswan should be
> uncrustified first.  This would ensure that the formatting was always
> consistent.  I strongly support this.
>
> == idempotence ==
>
> Unfortunately, uncrustify doesn't seem to be idempotent:
> 	uncrustify(libreswan) != uncrustify(uncrustify(libreswan))
>
> This concerns me.  It puts in doubt the idea of always uncrustifying.
>
> I wonder if uncrustify reaches a fixed point, or if the source changes
> with each application.  Is the format getting better each time?  I doubt
> it.
>
>
> == mangling macro arguments containing IF ==
>
> Kim found that certain calls of the DBG macro, ones where the argument
> contains an if statement, confuse uncrustify.  To work around this, he
> patched the source to eliminate these statements, uncrustified, and
> then put them back.  This is only good as a manual process -- the
> temporary patches are fragile.
>
> I'm testing a source code change that avoids the problem and doesn't
> make the source code too much uglier.
>
>
> == Stylistic changes I'd prefer ==
>
> - when an expression is broken by an newline, normally at a binary
>    operator, put the operator at the start of the new line.
>
>    This makes the structure of the program evident by scanning
>    only indentation and the leftmost token on a line.
>
>    In the case of control expressions for IF, WHILE, etc.
>    the new line should be indented the same as the start of the
>    statement.
>
> 	if (blah blah blah
> 	&& yack yack yack)
> 	{
> 		/* open brace is on its own line in this case */
> 	}
>
> - I prefer a blank line after "break" or "return" not immediately
>    followed by closing brace.  This highlights the control-flow
>    exception.
>
>    	if (ugh != NULL)
> 		return ugh;
>
> 	/* all seems well */
>
> - if an IF, WHILE, etc body is long enough to be broken, it should
>    probably be braced.
>
>    Comment lines too mean braces should be used:
>
> 	if (silly)
> 		/* should be braced */
> 		silly++;
>
> - if one side of an IF-ELSE has a brace, probably they both should.
>    A funny worst-case example is programs/pluto/connections.c line 904.
>
> - I suspect we should prevent uncrustify removing braces, even if the
>    transformation is correct.  They were probably placed there for a
>    readability reason.
>
> - I don't much like the way parameter declarations are indented but I
>    don't think it matters much.
>
> More to come.
> _______________________________________________
> Swan mailing list
> Swan at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan
>