On Fri, Dec 23, 2016 at 2:39 PM, Steffan Karger <stef...@karger.me> 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.)
Do we have to be so strict about the coding style which itself is so
ill-defined?
I'll happily adhere to any style if that becomes a requirement for patches,
but
overly depending on an external tool to ensure a style that's not even
precisely
defined by the CodeStyle page (except by the uncrustify config) sounds lame
to me.
Aiming towards a uniform coding style is a good goal. Requiring that
patches should be invariant under "uncrustify -c some-config" is not, imho.
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/intel
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
