[Swan] initial thoughts on uncrustifying libreswan

[David McCullough]

Philippe Vouters wrote the following:
> I agree the Linux kernel looks well written. However from an outside
> point of view as a code reader, the Linux kernel source reading has
> been of strictly no help for me to understand and correct the kernel
> oops my users and I faced and that I describe at http://vouters.dyndns.org/tima/Linux-drivers-Troubleshooting_a_oops.html.
> In this case and for me there has been lots of intuition deployed
> with absolutely no certainty.
> 
> I much prefer reading Libreswan code with its numerous DBG_logs
> rather than the Linux kernel code. Thery make follow the story far
> more easy.

I am talking about code formating only,  the logging in openswan is it's
biggest advantage,  especially in the kernel where traditionally there is
nothing.  I don't want the logging removed :-)

Cheers,
Davidm

> On 05/23/2013 12:35 PM, David McCullough wrote:
> >D. Hugh Redelmeier wrote the following:
> >>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.
> >Using a tool to get started sounds like a good idea, followed by
> >hand tweaking and rejection of poorly formated changes from there.
> >I don't it needs to be run continuously.
> >
> >>== 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 ==
> >Agree with almost everything said here.
> >
> >My preference is the Linux kernel style whenever possible.
> >
> >>- 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 don't like this personally.
> >
> >>- 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.
> >Agree with these.
> >
> >>- I don't much like the way parameter declarations are indented but I
> >>   don't think it matters much.
> >Again,  my preference is the Linux kernel style whenever possible.
> >Hopefully we can avoid another style ;-)
> >
> >Cheers,
> >Davidm
> >
> 
> _______________________________________________
> Swan mailing list
> Swan at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan

-- 
David McCullough,  davidm at spottygum.com,   Ph: 0410 560 763