Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Thu, Dec 15, 2016 at 8:00 AM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> > The only issue I noticed is that some functions, for loops and switch
> > statements have their opening braces on the same line
> > ssl.c line 272, tun.c line 697 and many such in the several files.
> > (try grep ") {$"  *.c).
> >
> > Probably needed nl_switch_brace, nl_fcall_brace etc in the config --
> > urgh.. Too many options is a blessing and bane at the same time..
>
> Agreed.  I think that we can do some minor reformatting changes later on
> if we decide to change some more options.  Those reformatting changes
> will most likely not be as invasive as this one has been.
>

There are also some multi-line comments with closing "*/" not on a line by
itself.

If we want to fix any of this I think this is the time -- massive
reformatting is not something to be done often.


> > This is not a show stopper for me. Could be handled manually in future
> > edits when code in the vicinity is touched..
>
> I will now write a new script which patch contributors should run
> against modified files before committing and submitting.  This should
> hopefully make it better in the long run.  But with that in mind, we
> will need to re-run the reformat-all.sh script each time we decide to
> modify uncrustify options.


We have to be cautious here. Running uncrustify on is not something we
should do frequently -- I thought this was a once in a lifetime
reformatting to be followed by some self-imposed discipline for future
patches guided by the CodeStyle page.

First, uncrustify is not idempotent, although running two or three times
appears to make the source settle into an invariant form. Sometimes
multiple runs slightly messes up some otherwise good formatting.  Secondly,
its an actively developed code so behaviour could change. In fact they
already fixed the segfault bug that I had reported upstream.

So I'd be wary of adopting a policy that patch submission be preceded by an
uncrustify run.

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

On 15/12/16 06:09, Selva Nair wrote:
> 
> On Wed, Dec 14, 2016 at 4:18 PM, David Sommerseth
>  > wrote:
> 
> On 13/12/16 22:42, David Sommerseth wrote:
> >
> > Hi all,
> >
> > So the first phase of the great reformatting is on its way.  I have
> > just pushed out a reformatting branch to the following
> > repositories:
> >
> > https://github.com:OpenVPN/openvpn.git
> > https://gitlab.com:openvpn/openvpn.git
> > git://git.code.sf.net/p/openvpn/openvpn
> 
> >
> 
> An updated reformatting branch have been pushed out.  If you have
> already fetched the it, git will most likely complain as to make
> these tests flow reasonably well without cluttering the git tree
> too much we have used forced pushes on this branch.
> 
> And again, the The Great Reformatting patch is PGP signed as well,
> which can be verified with 'git log --show-signature'.  The
> signing key is 0x86CF944C9671FDF2, which is the same as this mail
> is signed with.
> 
> 
> The reformatting looks pretty good for an automated process. The result
> agrees with independently running the script on master.
> 
> The only issue I noticed is that some functions, for loops and switch
> statements have their opening braces on the same line
> ssl.c line 272, tun.c line 697 and many such in the several files.
> (try grep ") {$"  *.c).
> 
> Probably needed nl_switch_brace, nl_fcall_brace etc in the config --
> urgh.. Too many options is a blessing and bane at the same time..

Agreed.  I think that we can do some minor reformatting changes later on
if we decide to change some more options.  Those reformatting changes
will most likely not be as invasive as this one has been.

> This is not a show stopper for me. Could be handled manually in future
> edits when code in the vicinity is touched..

I will now write a new script which patch contributors should run
against modified files before committing and submitting.  This should
hopefully make it better in the long run.  But with that in mind, we
will need to re-run the reformat-all.sh script each time we decide to
modify uncrustify options.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Wed, Dec 14, 2016 at 4:18 PM, David Sommerseth  wrote:

> On 13/12/16 22:42, David Sommerseth wrote:
> >
> > Hi all,
> >
> > So the first phase of the great reformatting is on its way.  I have
> > just pushed out a reformatting branch to the following
> > repositories:
> >
> > https://github.com:OpenVPN/openvpn.git
> > https://gitlab.com:openvpn/openvpn.git
> > git://git.code.sf.net/p/openvpn/openvpn
> >
>
> An updated reformatting branch have been pushed out.  If you have
> already fetched the it, git will most likely complain as to make
> these tests flow reasonably well without cluttering the git tree
> too much we have used forced pushes on this branch.
>
> And again, the The Great Reformatting patch is PGP signed as well,
> which can be verified with 'git log --show-signature'.  The
> signing key is 0x86CF944C9671FDF2, which is the same as this mail
> is signed with.


The reformatting looks pretty good for an automated process. The result
agrees with independently running the script on master.

The only issue I noticed is that some functions, for loops and switch
statements have their opening braces on the same line
ssl.c line 272, tun.c line 697 and many such in the several files.
(try grep ") {$"  *.c).

Probably needed nl_switch_brace, nl_fcall_brace etc in the config -- urgh..
Too many options is a blessing and bane at the same time..

This is not a show stopper for me. Could be handled manually in future
edits when code in the vicinity is touched..

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

Hi,

On 14-12-16 22:18, David Sommerseth wrote:
> On 13/12/16 22:42, David Sommerseth wrote:
>> So the first phase of the great reformatting is on its way.  I have
>> just pushed out a reformatting branch to the following 
>> repositories:
> 
>> https://github.com:OpenVPN/openvpn.git 
>> https://gitlab.com:openvpn/openvpn.git 
>> git://git.code.sf.net/p/openvpn/openvpn

> An updated reformatting branch have been pushed out.  If you have
> already fetched the it, git will most likely complain as to make
> these tests flow reasonably well without cluttering the git tree
> too much we have used forced pushes on this branch.

Great, getting really close now!  I just verified that commit
81d882d5302b8b647202a6893b57dfdc61fd6df2 is the result of running the
reformat-all.sh script from commit
2417d55c4945d491e528dd0e4cf24047da5ceae9 on that commit.

Scrolling through the changes almost everything looks good to me.  Some
details will need to be fixed when we touch the code, but in general
this looks like a *huge* improvement.

I also ran my usual tests: 'make check' including t_client for both
openssl and mbed TLS.

All this looks good, so ACK on merging
81d882d5302b8b647202a6893b57dfdc61fd6df2 into master.

-Steffan



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 13/12/16 22:42, David Sommerseth wrote:
> 
> Hi all,
> 
> So the first phase of the great reformatting is on its way.  I have
> just pushed out a reformatting branch to the following 
> repositories:
> 
> https://github.com:OpenVPN/openvpn.git 
> https://gitlab.com:openvpn/openvpn.git 
> git://git.code.sf.net/p/openvpn/openvpn
> 

An updated reformatting branch have been pushed out.  If you have
already fetched the it, git will most likely complain as to make
these tests flow reasonably well without cluttering the git tree
too much we have used forced pushes on this branch.

And again, the The Great Reformatting patch is PGP signed as well,
which can be verified with 'git log --show-signature'.  The
signing key is 0x86CF944C9671FDF2, which is the same as this mail
is signed with.


- -- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYUbceAAoJEIbPlEyWcf3yOnIQAKRMvwhlde6/X/oi4Tu94dRk
Czoqf9nVTeS/ejf/dwWuztVWR8hIfd0rf+gog6eTHBQrRSjvDHPyi62XTH2lkt9z
h+xOHZTOgL7oKyHWrfjiyGPxov2RUgz5HvMyH2nl2s6Pj3o9J/AkrfxFJbtMu/CR
YYk9w5QOn8/XfoulOgZgWh1JXwICGKUVQXFQRBoQpi7m3aVdFpRN/3uHmK+epWe/
ewEQd2kNYJQfPWCRuzDBTPCJfYmr9IDncB/mkvDGw7D08BdcFknOWeVmP1HHn4uM
XWsIqOqfilOf09dPzQnSfLxlgg0zGZW/z8wYBqKAZTM1aVO5z5Jtq6nqevWzB1UX
Nrlws0OaPlIsjGUzeL3BInGQevpYYKSHS3Acyz+cW+X42t3AIGTKUQSCgRQpFJnb
v4cRf+7c7h77t7mMIIsS7ghMJRGgog6ib2NJp07in4o75aHMFeuWHte4FmM3Osrk
vJ9MT1jUkxs1OxkFsxpnrhhzfY1UazCDmosMytoE+CaJzMIkB9HzlH77yR+jxnQK
lT3hineWNGgmEtTjmczZQJHHF68IHivfd7g07mS/xdvXWmRbkgUQNqcKOZujiaQV
OIfjvOkREtkbW1lJLinMeb3c+SW5DggKnkSwdo3yHOHvRfniPraQxVDbpDAd2dVu
0WPlpmIuHmBypIxNBpYc
=agzz
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Tue, Dec 13, 2016 at 5:08 PM, Steffan Karger  wrote:

> > This is based on the last master branch, with two additional commits.
> >  The first commit adds the reformatting script which does the complete
> > reformatting in a consistent way across the complete source tree.  The
> > second commit is massive, with the complete reformatting.
> >
> > Have fun and report back.
>
> Excellent.
>
> I just verified that 9f4993fbd4e6cd36c9c3f4661c78881b7e6121bc is indeed
> the result of running the formatting script commit in
> a084eb6cc294c7326395e01b6a5c14abb7ed4ee5 on that commit.



 sp_bool missing  <-- present in CodeStyle page

Surprisingly this has an effect only a couple of places (unlike sp_arith ;)
eg: syshead.h line 430

&(IP_PKTINFO)) || defined(IP_RECVDSTADDR)) && defined(HAVE_MSGHDR) \
&& defined(HAVE_CMSGHDR) && defined(HAVE_IOVEC) && defined(CMSG_FIRSTHDR) \
&& defined(CMSG_NXTHDR) && defined(HAVE_RECVMSG) && defined(HAVE_SENDMSG)

Otherwise looks as promised.

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

On 13-12-16 22:42, David Sommerseth wrote:
> So the first phase of the great reformatting is on its way.  I have
> just pushed out a reformatting branch to the following repositories:
> 
>https://github.com:OpenVPN/openvpn.git
>https://gitlab.com:openvpn/openvpn.git
>  git://git.code.sf.net/p/openvpn/openvpn
> 
> This is based on the last master branch, with two additional commits.
>  The first commit adds the reformatting script which does the complete
> reformatting in a consistent way across the complete source tree.  The
> second commit is massive, with the complete reformatting.
> 
> Have fun and report back.

Excellent.

I just verified that 9f4993fbd4e6cd36c9c3f4661c78881b7e6121bc is indeed
the result of running the formatting script commit in
a084eb6cc294c7326395e01b6a5c14abb7ed4ee5 on that commit.

-Steffan



signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1


Hi all,

So the first phase of the great reformatting is on its way.  I have
just pushed out a reformatting branch to the following repositories:

   https://github.com:OpenVPN/openvpn.git
   https://gitlab.com:openvpn/openvpn.git
 git://git.code.sf.net/p/openvpn/openvpn

This is based on the last master branch, with two additional commits.
 The first commit adds the reformatting script which does the complete
reformatting in a consistent way across the complete source tree.  The
second commit is massive, with the complete reformatting.

Have fun and report back.

Btw.  The reformatting commit is PGP signed using the same key this
mail is signed with.


- -- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc

-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBAgAGBQJYUGs0AAoJEIbPlEyWcf3yLMAP/R4vVszKHG8bQNAJYtUSKnUd
PFw+DVw+kyaiPnuzrCF1MWrCcV20uC9dTmWivI/7qLxWvk6NlcFTcy9ziXeOitK1
yZE3Xc1n3GsDnV6DBUbbsoHNS0PSJ6GldnyBMSXm7MuxOm1fZYQHems5B+tWA1IW
pA+alUkY+Z2U2VQ2XQbg4tIAwwXiiI9+AkIT+xHRxAiYlrMSJG/Mkw2hFuJCn9sB
8vtWU0h0ckFC60ouR7fsrcEyq307DhKEVmdgjYTv9sgTDlzxMIEsgWhS6PV1MqnB
FH9hKirQ8ATxLY2iE8cofBuD7wbcrxRSSksl9ko8Si7WOiVLq39Gf+G7sj1fBSeG
qB7WvEga32BuIsIJYbvLCBbXSUF7nDytyeqj6w4QLyzEcRyr+c+QRDKwTrLf/mKO
7oJY7KSz78DUefBX1u23njadBu64AKqG7PkWwOQPA/piPXWl0o8M7DeE/OcG+ZS3
Dt1YvJ0PgYjwVSk7oBqBK3NwloMAuHLKYH27V1ZjUmn8GkSho0Na0tAmsMOwHF+f
wlskxKdhDPWzfQXwl2MihxjyexsIKgousglnYMGjI7BImtnYxPRlS3TEmmF0MiFE
IvP5hWQepWinctVOdbsFObKe7MEfMPTBlrOcMpklifQlDFm+iRDNf4QYpxG7UBoE
gnqSLAqeLFjAbof94Crc
=eLCd
-END PGP SIGNATURE-

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

On 13/12/16 22:05, Selva Nair wrote:
> 
> On Tue, Dec 13, 2016 at 3:56 PM, David Sommerseth
>  > wrote:
> 
> Already at it.  Written a script to do everything in a consistent way,
> added the cmt_cpp_to_c=true (and sp_arith=add)  
> 
> 
> As Steffan mentioned we do had decided against sp_arith = add, but
> please do use pos_bool = Lead.

Yes will do.

But to be honest, I think we will end up manually editing quite some
places where sp_arith would do it for us.  But decided is decided.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Tue, Dec 13, 2016 at 3:56 PM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> Already at it.  Written a script to do everything in a consistent way,
> added the cmt_cpp_to_c=true (and sp_arith=add) 
>

As Steffan mentioned we do had decided against sp_arith = add, but please
do use pos_bool = Lead.

Thanks,

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Tue, Dec 13, 2016 at 3:59 PM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> If it breaks a boolean expression over more lines, the operator comes as
>  the lead character on the following line
>
> i = TEST1
>| TEST2
>| TEST3
>
> instead of
>
> i = TEST1 |
> TEST2 |
> TEST3
>

That is already handled by pos_arith=Lead. This pos_bool handles boolean
operators like && and ||

:)

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

On 13/12/16 21:10, Steffan Karger wrote:
> Hi,
> 
> On 13-12-16 20:54, Selva Nair wrote:
>>
>> On Tue, Dec 13, 2016 at 1:55 PM, Steffan Karger > > wrote:
>>
>> >
>> > On 09/12/16 22:27, Steffan Karger wrote:
>> > >
>> > > Sounds like we have a final config on the CodeStyle page now.  
>> Are we
>> > > ready to run it on all code now, and publish a reformat branch?
>> > >
>> >
>> > Agreed.  I can do this later this night.
>> >
>> > Wait a minute we may need cmt_cpp_to_c=true  -- I saw some C++ comments
>> > (added by uncrustify) in header files against #endif etc..
>>
>> Indeed, tested and added to the CodeStyle page.  Anyone up for creating
>> the reformatting branch?
>>
>>
>> I did some test over the weekend. For what its worth just pushed the branch 
>> to https://github.com/selvanair/openvpn/tree/uncrustify
>>
>> This uses all options at the CodeStyle page plus
>> sp_bool=add
> 
> This makes sense.
> 
>> sp_arith=add
> 
> I think we agreed on IRC to not touch arithmetic spacing, because
> clarity really depends on the code.

Ouch ... there were a few places, where this really needed to be cleaned
up, otherwise it was really not good.

>> pos_bool=lead
> 
> What does this do exactly?  The description "The position of boolean
> operators in wrapped expressions" from --show-config doesn't ring a bell

If it breaks a boolean expression over more lines, the operator comes as
 the lead character on the following line

i = TEST1
   | TEST2
   | TEST3

instead of

i = TEST1 |
TEST2 |
TEST3

-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

On 13/12/16 19:55, Steffan Karger wrote:
> 
> On 09-12-16 22:44, Selva Nair wrote:
>> On Fri, Dec 9, 2016 at 4:39 PM, David Sommerseth
>> > > wrote:
>>
>> On 09/12/16 22:27, Steffan Karger wrote:
>> >
>> > Sounds like we have a final config on the CodeStyle page now.  Are we
>> > ready to run it on all code now, and publish a reformat branch?
>> >
>>
>> Agreed.  I can do this later this night.
>>
>> Wait a minute we may need cmt_cpp_to_c=true  -- I saw some C++ comments
>> (added by uncrustify) in header files against #endif etc..
> 
> Indeed, tested and added to the CodeStyle page.  Anyone up for creating
> the reformatting branch?

Already at it.  Written a script to do everything in a consistent way,
added the cmt_cpp_to_c=true (and sp_arith=add)  just reviewing that
things isn't too bad ... I do see we have some files which will need
some refactoring to be readable, mostly due to our 80 chars limit.  But
looking good ... hope to have something out within a few hours.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

Hi,

On 13-12-16 21:22, Selva Nair wrote:
> > pos_bool=lead
> 
> What does this do exactly?  The description "The position of boolean
> operators in wrapped expressions" from --show-config doesn't ring a bell
> for me.
> 
> This extends the pos_arith=lead (Knuth style) to booleans too so that we get
> 
> x = first_condition 
>   && second_condition
>   && third_condition
> 
> instead of leaving && at the tail of previous line... Not a big thing,
> but I thought it looks better.

Agreed.  I added pos_bool=Lead to the CodeStyle page, sp_bool=add was
already there.

Anything else holding back the reformatting branch?  I think we should
really move on with this, if we want to have the chance to do any
additional manual fixes after running uncrustify.

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Tue, Dec 13, 2016 at 3:10 PM, Steffan Karger  wrote:

> > sp_bool=add
>
> This makes sense



> > sp_arith=add
>
> I think we agreed on IRC to not touch arithmetic spacing, because
> clarity really depends on the code.
>

Forgot about that discussion. Seeing lines like
|FORMAT_MESSAGE_ALLOCATE_BUFFER, looked like a good idea to add that to get
a space after '|'. But it does changes 'buf+7' to 'buf + 7' etc. which is
not ideal. OK, let's leave that out.


> > pos_bool=lead
>
> What does this do exactly?  The description "The position of boolean
> operators in wrapped expressions" from --show-config doesn't ring a bell
> for me.
>

This extends the pos_arith=lead (Knuth style) to booleans too so that we get

x = first_condition
  && second_condition
  && third_condition

instead of leaving && at the tail of previous line... Not a big thing, but
I thought it looks better.

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

Hi,

On 13-12-16 20:54, Selva Nair wrote:
> 
> On Tue, Dec 13, 2016 at 1:55 PM, Steffan Karger  > wrote:
> 
> >
> > On 09/12/16 22:27, Steffan Karger wrote:
> > >
> > > Sounds like we have a final config on the CodeStyle page now.  
> Are we
> > > ready to run it on all code now, and publish a reformat branch?
> > >
> >
> > Agreed.  I can do this later this night.
> >
> > Wait a minute we may need cmt_cpp_to_c=true  -- I saw some C++ comments
> > (added by uncrustify) in header files against #endif etc..
> 
> Indeed, tested and added to the CodeStyle page.  Anyone up for creating
> the reformatting branch?
> 
> 
> I did some test over the weekend. For what its worth just pushed the branch 
> to https://github.com/selvanair/openvpn/tree/uncrustify
> 
> This uses all options at the CodeStyle page plus
> sp_bool=add

This makes sense.

> sp_arith=add

I think we agreed on IRC to not touch arithmetic spacing, because
clarity really depends on the code.

> pos_bool=lead

What does this do exactly?  The description "The position of boolean
operators in wrapped expressions" from --show-config doesn't ring a bell
for me.

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Tue, Dec 13, 2016 at 1:55 PM, Steffan Karger  wrote:

> >
> > On 09/12/16 22:27, Steffan Karger wrote:
> > >
> > > Sounds like we have a final config on the CodeStyle page now.  Are
> we
> > > ready to run it on all code now, and publish a reformat branch?
> > >
> >
> > Agreed.  I can do this later this night.
> >
> > Wait a minute we may need cmt_cpp_to_c=true  -- I saw some C++ comments
> > (added by uncrustify) in header files against #endif etc..
>
> Indeed, tested and added to the CodeStyle page.  Anyone up for creating
> the reformatting branch?


I did some test over the weekend. For what its worth just pushed the branch
to https://github.com/selvanair/openvpn/tree/uncrustify

This uses all options at the CodeStyle page plus
sp_bool=add
sp_arith=add
pos_bool=lead

which I found to have some "beautifying" effects :)

Notes:
One file (include/openvpn-plugins.h) caused uncrustify to segfault
so was let out (haven't yet got time to investigate).

More details of the command used is here:
https://gist.github.com/selvanair/bdd3f6151e72a428055d5d5e87cdbe2b
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

On 09-12-16 22:44, Selva Nair wrote:
> On Fri, Dec 9, 2016 at 4:39 PM, David Sommerseth
>  > wrote:
> 
> On 09/12/16 22:27, Steffan Karger wrote:
> >
> > Sounds like we have a final config on the CodeStyle page now.  Are we
> > ready to run it on all code now, and publish a reformat branch?
> >
> 
> Agreed.  I can do this later this night.
> 
> Wait a minute we may need cmt_cpp_to_c=true  -- I saw some C++ comments
> (added by uncrustify) in header files against #endif etc..

Indeed, tested and added to the CodeStyle page.  Anyone up for creating
the reformatting branch?

-Steffan

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Fri, Dec 9, 2016 at 4:39 PM, David Sommerseth <
open...@sf.lists.topphemmelig.net> wrote:

> On 09/12/16 22:27, Steffan Karger wrote:
> >
> > Sounds like we have a final config on the CodeStyle page now.  Are we
> > ready to run it on all code now, and publish a reformat branch?
> >
>
> Agreed.  I can do this later this night.



Wait a minute we may need cmt_cpp_to_c=true  -- I saw some C++ comments
(added by uncrustify) in header files against #endif etc..

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

On 09/12/16 22:27, Steffan Karger wrote:
> 
> Sounds like we have a final config on the CodeStyle page now.  Are we
> ready to run it on all code now, and publish a reformat branch?
> 

Agreed.  I can do this later this night.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

On 9 December 2016 at 21:43, Selva Nair  wrote:
> On Fri, Dec 9, 2016 at 8:41 AM, Steffan Karger  wrote:
>> On 9 December 2016 at 00:14, David Sommerseth
>>  wrote:
>> > I just spotted in ssl.c that we need sp_assign=add.
>> >
>> > [ ssl.c, tls1_PRF() ]
>> > len = slen/2;
>> > S1 = sec;
>> > S2 = &(sec[len]);
>> > len += (slen&1); /* add for odd, make longer */
>> >
>> > I believe we've agreed on spaces around assignments.
>>
>> Hm, I'd be fine with that, but I'm afraid there are too many such
>> details to bikeshed about.  Once we've fixed the curious newlines
>> Selva mentionded (sorry, not sure when I can look at that), I think we
>> should stop the discussion and move forward.  Just 5 more days until
>> rc2 (where the reformatting should be done).
>
> Agreed.  I'll make no more noise :)
>
> To troubleshoot the extra newlines in places like line 130 of the file you
> uploaded
> https://paste.fedoraproject.org/502063/36953148
> I ran uncrustify using the options listed at our CodeStyle page and all look
> good and nice. No such extra newlines. By the way sp_between_byref is not a
> valid option.

Hm, indeed.  Must have been an artifact of running uncrustify multiple
times or something like that.  I removed sp_between_byref.

Sounds like we have a final config on the CodeStyle page now.  Are we
ready to run it on all code now, and publish a reformat branch?

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Fri, Dec 9, 2016 at 8:41 AM, Steffan Karger  wrote:

>
> On 9 December 2016 at 00:14, David Sommerseth
>  wrote:
> > I just spotted in ssl.c that we need sp_assign=add.
> >
> > [ ssl.c, tls1_PRF() ]
> > len = slen/2;
> > S1 = sec;
> > S2 = &(sec[len]);
> > len += (slen&1); /* add for odd, make longer */
> >
> > I believe we've agreed on spaces around assignments.
>
> Hm, I'd be fine with that, but I'm afraid there are too many such
> details to bikeshed about.  Once we've fixed the curious newlines
> Selva mentionded (sorry, not sure when I can look at that), I think we
> should stop the discussion and move forward.  Just 5 more days until
> rc2 (where the reformatting should be done).



Agreed.  I'll make no more noise :)

To troubleshoot the extra newlines in places like line 130 of the file you
uploaded
https://paste.fedoraproject.org/502063/36953148
I ran uncrustify using the options listed at our CodeStyle page and all
look good and nice. No such extra newlines. By the way sp_between_byref is
not a valid option.

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

Hi,

On 9 December 2016 at 00:14, David Sommerseth
 wrote:
> I just spotted in ssl.c that we need sp_assign=add.
>
> [ ssl.c, tls1_PRF() ]
> len = slen/2;
> S1 = sec;
> S2 = &(sec[len]);
> len += (slen&1); /* add for odd, make longer */
>
> I believe we've agreed on spaces around assignments.

Hm, I'd be fine with that, but I'm afraid there are too many such
details to bikeshed about.  Once we've fixed the curious newlines
Selva mentionded (sorry, not sure when I can look at that), I think we
should stop the discussion and move forward.  Just 5 more days until
rc2 (where the reformatting should be done).

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Gert Doering]

Hi,

On Fri, Dec 09, 2016 at 08:24:24AM -0500, Selva Nair wrote:
> On Fri, Dec 9, 2016 at 2:42 AM, Gert Doering  wrote:
> 
> >   if (a>0)
> >  { do_this(); }
> >   else
> >  { do_that(); }
> >
> 
> In such cases I would normally skip all braces, in spite of all the
> arguments against it... But that's just me.

We've decided that we always want braces, "because apple goto bug",
but also because there's a slippery slope towards surprising bugs.


if (foo)
   return 3;
else
   if (bar)
   do_something(); return 2;
   else
   return 4;

(maybe a non-convincing example because "obviously buggy")

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Fri, Dec 9, 2016 at 2:42 AM, Gert Doering  wrote:

>   if (a>0)
>  { do_this(); }
>   else
>  { do_that(); }
>

In such cases I would normally skip all braces, in spite of all the
arguments against it... But that's just me.

That said the proposed re-formatting looks super good to me and uncrustify
is doing an amazing job. Kudos to all who narrowed down the options.

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Gert Doering]

Hi,

On Thu, Dec 08, 2016 at 05:04:53PM -0500, Selva Nair wrote:
> if ()
> {
>...
> }
> followed by
> else if (xxx) {
>  
> }
> 
> looks inconsitent and not pretty. 

Thanks, so I'm not alone with this :-)

> Why not just stick to:
> 
> if (carefree())
> {
>follow_me();
> }
> else if (cautious())
> {
>as_you_wish();
> }
> else
> {
>abort();
> }
> 
> Looks easier to read.

This!

Maybe it's just my OCD speaking, but 

  if (something)
  {
  this();
  } else {
  that();
  }

really makes my skin crawl - I need the "i"s dotted, the "t"s crossed,
and braces aligned - vertically or horizontally, but not "diagonally".

So this would work for me, for "trivial conditionals" (= short condition,
non-crammed single-line statements):

  if (a>0)
 { do_this(); }
  else
 { do_that(); }

but since we agred on "braces get their own line", it's out as well.

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

On 08/12/16 23:45, David Sommerseth wrote:
> On 08/12/16 22:56, Selva Nair wrote:
>> Hi,
>>
>> On Thu, Dec 8, 2016 at 4:15 PM, David Sommerseth
>> > > wrote:
>>
>>
>> This is fairly inline with the initial agreement from the 2014 Munich
>> hackathon [3] and the summarized CodeStyle wiki page [4].
>>
>> This proposal allows for the "open bracing" question, which allows
>> '} else {'  and '} else if () {'.
>>
>>
>> So now the bike shed debate opens up ... Is this something we can get
>> consensus around?
>>
>> And a few examples of reformatted files:
>>
>>  tun.c: > >
>>  ssl.c: > >
>>
>>
>> Looks excellent.
>>
>> Yes, responding to the call for bikeshedding , some questions/comments:
>>
>> Reformatted tun.c has at least two instances of char* xxx, we want char
>> *xxx, right? So why not add some sp_xxx_ptr_star options.
> 
> Good catch!  tun.c and ssl.c are the biggest files we have ... so there
> might be some details we've not spotted.  Uncrustify doesn't change
> things we don't tell it to change, so we just haven't spotted this.
> 
> I agree, we should change to this.
> 
>> Less of a concern at least for me is function_name (...) and
>> function_name(...) both appears to be allowed. Just because we can do
>> it, why not add some sp_func__paren options to normalize those..
> 
> I agree to consistency.  And Steffan already suggested what I was about
> to suggest.  I'll send an updated config file soonish too.

Steffan was more clever and updated the wiki page.

I just spotted in ssl.c that we need sp_assign=add.

[ ssl.c, tls1_PRF() ]
len = slen/2;
S1 = sec;
S2 = &(sec[len]);
len += (slen&1); /* add for odd, make longer */

I believe we've agreed on spaces around assignments.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

On 08/12/16 22:56, Selva Nair wrote:
> Hi,
> 
> On Thu, Dec 8, 2016 at 4:15 PM, David Sommerseth
>  > wrote:
> 
> 
> This is fairly inline with the initial agreement from the 2014 Munich
> hackathon [3] and the summarized CodeStyle wiki page [4].
> 
> This proposal allows for the "open bracing" question, which allows
> '} else {'  and '} else if () {'.
> 
> 
> So now the bike shed debate opens up ... Is this something we can get
> consensus around?
> 
> And a few examples of reformatted files:
> 
>  tun.c:  >
>  ssl.c:  >
> 
> 
> Looks excellent.
> 
> Yes, responding to the call for bikeshedding , some questions/comments:
> 
> Reformatted tun.c has at least two instances of char* xxx, we want char
> *xxx, right? So why not add some sp_xxx_ptr_star options.

Good catch!  tun.c and ssl.c are the biggest files we have ... so there
might be some details we've not spotted.  Uncrustify doesn't change
things we don't tell it to change, so we just haven't spotted this.

I agree, we should change to this.

> Less of a concern at least for me is function_name (...) and
> function_name(...) both appears to be allowed. Just because we can do
> it, why not add some sp_func__paren options to normalize those..

I agree to consistency.  And Steffan already suggested what I was about
to suggest.  I'll send an updated config file soonish too.


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc




signature.asc
Description: OpenPGP digital signature
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

Hi,

On 08-12-16 23:24, Steffan Karger wrote:
> On 08-12-16 22:56, Selva Nair wrote:
>> Reformatted tun.c has at least two instances of char* xxx, we want char
>> *xxx, right? So why not add some sp_xxx_ptr_star options.
> 
> We settled for char *xxx yesterday, so I'll add that.
> 
>> Less of a concern at least for me is function_name (...) and
>> function_name(...) both appears to be allowed. Just because we can do
>> it, why not add some sp_func__paren options to normalize those.
> 
> Agreed.  Allman (at least according to the wikipedia examples) uses
> funcion_name(), so I'll add that too.
> 
> I'll update the options on the CodeStyle page.

Done, new tun.c example here:
https://paste.fedoraproject.org/502063/36953148/

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

On 08-12-16 22:56, Selva Nair wrote:
> Reformatted tun.c has at least two instances of char* xxx, we want char
> *xxx, right? So why not add some sp_xxx_ptr_star options.

We settled for char *xxx yesterday, so I'll add that.

> Less of a concern at least for me is function_name (...) and
> function_name(...) both appears to be allowed. Just because we can do
> it, why not add some sp_func__paren options to normalize those.

Agreed.  Allman (at least according to the wikipedia examples) uses
funcion_name(), so I'll add that too.

I'll update the options on the CodeStyle page.

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

On Thu, Dec 8, 2016 at 4:50 PM, Steffan Karger  wrote:

> .
>
> > This proposal allows for the "open bracing" question, which allows
> > '} else {'  and '} else if () {'.
> >
> > So now the bike shed debate opens up ... Is this something we can get
> > consensus around?
>
> Even though I asked for this on the CodeStyle page, I have to agree with
> Gert's argument on IRC:  we settled for Allman in Munich, so we should
> not re-open the bikeshed (even more ;) ).  So, even though it's not my
> personal preference, I argue we should not allow this.  Apologies for
> the noise...
>

I forgot to mention that, which was the only real comemnt I had:

if ()
{
   ...
}
followed by
else if (xxx) {
 
}

looks inconsitent and not pretty. Why not just stick to:

if (carefree())
{
   follow_me();
}
else if (cautious())
{
   as_you_wish();
}
else
{
   abort();
}

Looks easier to read.

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Selva Nair]

Hi,

On Thu, Dec 8, 2016 at 4:15 PM, David Sommerseth  wrote:

>
> This is fairly inline with the initial agreement from the 2014 Munich
> hackathon [3] and the summarized CodeStyle wiki page [4].
>
> This proposal allows for the "open bracing" question, which allows
> '} else {'  and '} else if () {'.
>
>
> So now the bike shed debate opens up ... Is this something we can get
> consensus around?
>
> And a few examples of reformatted files:
>
>  tun.c: 
>  ssl.c: 


Looks excellent.

Yes, responding to the call for bikeshedding , some questions/comments:

Reformatted tun.c has at least two instances of char* xxx, we want char
*xxx, right? So why not add some sp_xxx_ptr_star options.

Less of a concern at least for me is function_name (...) and
function_name(...) both appears to be allowed. Just because we can do it,
why not add some sp_func__paren options to normalize those..

Selva
--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Coding style clean-up ... phase 1

[Steffan Karger]

On 08-12-16 22:15, David Sommerseth wrote:
> This is fairly inline with the initial agreement from the 2014 Munich
> hackathon [3] and the summarized CodeStyle wiki page [4].

Thanks!  The consistency of the reformatted examples is an amazing
contrast to our current code base.

> This proposal allows for the "open bracing" question, which allows
> '} else {'  and '} else if () {'.
> 
> So now the bike shed debate opens up ... Is this something we can get
> consensus around?

Even though I asked for this on the CodeStyle page, I have to agree with
Gert's argument on IRC:  we settled for Allman in Munich, so we should
not re-open the bikeshed (even more ;) ).  So, even though it's not my
personal preference, I argue we should not allow this.  Apologies for
the noise...

-Steffan

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] Coding style clean-up ... phase 1

[David Sommerseth]

Hi all developers!

Gert did quite some efforts preparing the grounds for an uncrustify [1]
configuration in our CodeStyle wiki page.  I have also done several
tests with both astyle [2] and uncrustify, to see what would be most
simple for us to use and cover our needs.  I ended up with throwing out
astyle in the end, as it seems to do some weird stuff in some nested
if() situations (it didn't add the expected braces at the expected
places).  Uncrustify on the other hand is very flexible with an option
set which makes the numerous OpenVPN options look like a short list.

Next thing was that I spent time tweaking Gert's proposal and reviewing
it together with Steffan and Arne.  And I think we have a reasonable
configuration now.  And we're not even using that many options :)

--
indent_columns=4
indent_with_tabs=0
align_with_tabs=false
cmt_convert_tab_to_spaces=true
indent_braces=false
indent_else_if=false
indent_switch_case=4
indent_label=1
pp_space=remove
sp_before_sparen=add
sp_inside_sparen=remove
sp_cond_colon=add
sp_cond_question=add
sp_bool=add
sp_else_brace=add
sp_brace_else=add
pos_arith=Lead
nl_if_brace=add
nl_brace_else=remove
nl_elseif_brace=remove
nl_else_brace=remove
nl_else_if=remove
nl_func_type_name=add
nl_before_case=true
nl_assign_leave_one_liners=true
nl_enum_leave_one_liners=true
nl_brace_fparen=add
nl_max=4
nl_after_func_proto=2
code_width=80
ls_code_width=false
mod_full_brace_if=add
mod_full_brace_if_chain=false
mod_add_long_ifdef_endif_comment=20
mod_add_long_ifdef_else_comment=5
mod_remove_extra_semicolon=true
cmt_c_nl_end=true
cmt_star_cont=true
--

This is fairly inline with the initial agreement from the 2014 Munich
hackathon [3] and the summarized CodeStyle wiki page [4].

This proposal allows for the "open bracing" question, which allows
'} else {'  and '} else if () {'.


So now the bike shed debate opens up ... Is this something we can get
consensus around?

And a few examples of reformatted files:

 tun.c: 
 ssl.c: 


[1] 
[2] 
[3] 
[4] 


-- 
kind regards,

David Sommerseth
OpenVPN Technologies, Inc


--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today.http://sdm.link/xeonphi
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel