Hi,

On 23-12-16 17:17, David Sommerseth wrote:
> On 21/12/16 23:03, Steffan Karger wrote:
>> On 21 December 2016 at 22:09, David Sommerseth
>> <open...@sf.lists.topphemmelig.net> wrote:
>>> On 18/12/16 19:26, Steffan Karger wrote:
>>>> Now that we have touched each and every file anyway, I decided to go over
>>>> the code I regularly work with and reformat it some more by hand.  This is
>>>> how for I got today, and is a large enough patch I think.
>>>>
>>>> This commit is mostly just reordering and changing whitespace, with one
>>>> exception: it removes the #if 0 around some debugging code in
>>>> read_key_file(), and now always print the debugging if D_SHOW_KEYS is
>>>> enabled.
>>>>
>>>> This patch is best reviewed with something like
>>>> 'git diff --ignore-space-change'.
>>>>
>>>> Signed-off-by: Steffan Karger <stef...@karger.me>
>>>> ---
>>>> v2: fix wrong indent, add more 'for () {' -> 'for ()\n{' fixes.
>>>>
>>>>  src/openvpn/crypto.c         | 425 
>>>> ++++++++++++++++++++++---------------------
>>>>  src/openvpn/crypto.h         |  27 ++-
>>>>  src/openvpn/crypto_mbedtls.c |  63 ++++---
>>>>  src/openvpn/crypto_openssl.c |  38 ++--
>>>>  src/openvpn/cryptoapi.c      | 101 ++++++----
>>>>  5 files changed, 356 insertions(+), 298 deletions(-)
>>>>
>>> [...snip...]
>>>
>>> Hmmm ... I like that we're trying to clean up the formatting further.
>>> But I'm not too happy that uncrustify seems to disagree slightly ...
>>> See the attached patch what happened after applying your patch and then
>>> running:
>>>
>>>    $ uncrustify --no-backup -l C $files
>>>
>>> We should either see if our uncrustify config is correct or need slight
>>> adjustments (without needing to re-run it on the complete tree once again)
>>
>> This seems to be due to 2 things:
>>
>> 1) I adhered to the CodeStyle page 'When lines exceed this length,
>> wrap them using a double indent (ie 8 spaces)', while the crustify
>> config uses a single indent.  Since this double indent was somewhat
>> arbitrary, I think we should just change the CodeStyle page to single
>> indent.
> 
> And this goes against what was agreed upon in the Munich 2014 hackathon:
> 
>   o line length?
>      - breaking a line in the middle: align to opening (round) brackets
>        in line above
> 
>   <http://community.openvpn.net/openvpn/wiki/MunichHackathon2014>

That's about 2), not 1), but I get the point.  As before, I'll just have
to accept the "we decided on this before argument", and deal with it.
(I'm not sure how to keep the code readable with this requirement, but I
guess we'll find out.)

> This is yet another thing which have appeared into the CodeStyle wiki
> page without much discussion, as far as I am aware.

To fair about the CodeStyle page, I created it a number of weeks before
the rc2 deadline to get the reformatting ball rolling and *explicitly*
asked for feedback and additions on multiple occasions (IRC, meeting
agendas).

-Steffan

Attachment: 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/intel
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to