Hi,
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.
2) Uncrustify insists on either 'align function parameters' or 'indent
function parameters'. There is no 'ignore' for this option. The
current config uses 'align', which is fine in a lot of places, but in
some cases (such as the ones I've changed) it impairs readability.
Point 2 makes me believe that we should not enforce the code style
with a tool, but rather just with eyes during patch review. There's
just too many places where some freedom can improve readability.
Alternatively, if everyone thinks that 'indent' is better than
'align', we can change the config and postpone this discussion until
the next time where we run into something ;)
-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/intel
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
