Here's the summary of today's IRC meeting.



Place: #openvpn-meeting on irc.freenode.net
Date: Wednesday 7th Dec 2016
Time: 20:00 CET (19:00 UTC)

Planned meeting topics for this meeting were here:


The next meeting has been scheduled to a week from now (Wed 14th December), at the same time as today.

Your local meeting time is easy to check from services such as



cron, dazo, janjust, krzee, mattock, selvanair and syzzer participated in this meeting.


Discussed patchwork briefly:


Noted that patchwork has not yet been installed, and that will not change before 2.4.0 is out.


Discussed the OpenVPN 2.4_rc2 release:


Two problems have been found that need to be fixed for 2.4_rc2:

- systemd + --chroot is an unhappy combination
- the --mtu-disc option is  broken on server side

The first issue has a pending patch, and work on the second one has started.


Discussed the --no-iv option is OpenVPN. It was agreed that the performance gains (~1.5% with packet size of 1500 bytes) are not big enough to warrant keeping this (insecure) option around.

Syzzer sent a "deprecate --no-iv" patch to the list during the meeting.


Discussed OpenVPN 2.4 security audits by OSTIF.org and PIA:


These two organizations have so far been working on roughly the same goal without knowledge of each other. It was agreed that we should ensure that they remain in sync so that their work does not overlap too much.


Discussed the "great code reformatting" planned for OpenVPN 2.4_rc2. The reformatted code will follow the Allman style:


All tweaks to the coding style were documented on the above page.

Agreed to use the reformatting approach suggested by syzzer:

(1) define the exact tool, version and flags we use for the formatting, (2) get an ACK on that on the list
(3) push a commit with just running that on the codebase
(4) get some public ACKs on the commit SHA to confirm this is exactly running the tool
(5) send style fixup patches following the default procedure

Also agreed to dazo's suggestion of adding a tools/script directory which has a simple script running the indenting properly, and which can then be used by patch contributors as well.

Also agreed to create a reformatting branch in Git.


Discussed removing user choices from the Windows installer:


It was agreed that refactoring the installer should be taken care of after currently open PRs have been merged. The goal is to get a refactored installer in OpenVPN 2.4_rc2 (due 15th Dec).


Full chatlog has been attached to this email.

Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

(21:01:09) mattock: meeting time
(21:01:31) ***dazo brb
(21:03:18) cron2: burbs
(21:04:04) krzee: gday
(21:04:08) mattock: let me know when you've burbed enough :P
(21:04:11) mattock: hi krzee!
(21:04:16) ***cron2 wants beer
(21:04:42) ***dazo is here
(21:04:50) syzzer: evening
(21:04:59) mattock: https://community.openvpn.net/openvpn/wiki/Topics-2016-12-07
(21:05:01) vpnHelper: Title: Topics-2016-12-07 – OpenVPN Community (at 
(21:05:32) mattock: hi all!
(21:05:34) cron2: 2. has been answered on the list already "nothing happened, 
everyone too busy, later!" - check
(21:05:47) krzee: !beer
(21:05:55) mattock: yes, patchwork
(21:06:14) mattock: no progress there
(21:06:18) cron2: (as long as it's not *forgotten*...)
(21:06:20) mattock: 2.4 next?
(21:06:29) dazo: yes
(21:07:12) dazo: we have two issues located .... systemd + --chroot is an 
unhappy combination, patch resolving it for 2.4_rc2/0 is on the list ... and 
then --mtu-disc is broken on server side
(21:07:31) dazo: (which we were discussing on #openvpn-devel right before this 
(21:07:38) cron2: and AES-OFB ASSERT()s on reneg
(21:07:43) dazo: right!
(21:07:49) cron2: (patch on the list as well)
(21:08:04) syzzer: *-OFB/CFB, actually
(21:08:09) syzzer: but yeah
(21:09:25) dazo: anyone want to review syzzer patch?  Or should I add that to 
my TODO list as well?
(21:10:02) cron2: I have no idea what CO_PACKET_ID_LONG_FORM does or why it 
breaks AES-OFB/CFB, but besides that, it looks logical...
(21:11:11) dazo: I'm planning to work another 3-4 hours today ... and I'd like 
to get all those issues for rc2 closed as soon as possible and I have a chance 
to have a look at it today
(21:11:18) cron2: ah, I should just read the lines above
(21:11:52) cron2: syzzer: so why does it ASSERT?  Or was that "it gets confused 
and that might lead to all the most exciting things"
(21:12:12) syzzer: cron2: it gets confused
(21:12:15) cron2: dazo: as a side note, we're not releasing rc2 tomorrow
(21:12:30) mattock: next week
(21:12:47) mattock: according to the original schedule
(21:12:50) mattock: https://community.openvpn.net/openvpn/wiki/StatusOfOpenvpn24
(21:12:52) vpnHelper: Title: StatusOfOpenvpn24 – OpenVPN Community (at 
(21:12:54) syzzer: because in in the cfb/ofb case, it basically sets teh flags 
to 0 (or *_LONG_FORM)
(21:13:00) cron2: (so no need to pull an all-nighter tonight, better get enough 
(21:13:02) dazo: cron2: I know ... but I'd like to get a "git master" spin out 
on at least one or two of my VPN servers ... to have some extra testing time in 
(21:13:20) syzzer: which borkes if the CFB.OFB code does ASSERT(flags & USE_IV)
(21:13:28) cron2: well, that is comparably easy :)
(21:13:33) cron2: ic.  makes sense
(21:14:02) cron2: (and it would also explode if someone pushes AES-GCM with 
--no-iv set before)
(21:14:09) syzzer: exactly
(21:14:28) syzzer: more nails on the coffin of --no-iv!
(21:15:03) syzzer: that sh*t merely complicates things
(21:15:32) cron2: I'll fix a few typos in the commit message on the fly, and 
change the trac bit to "Trac: #784" ("new format as ordered by chief dazo")
(21:15:46) syzzer: sure
(21:16:22) dazo: cron2: the "Trac:" formatting will also be caught by 
git-ackam/git-regen-ackmsg too
(21:16:23) krzee: do people actively use --no-iv?
(21:16:34) syzzer: I hope not...
(21:16:45) krzee: thats what i was thinking :D
(21:16:54) krzee: like, isnt there a reason for is!?
(21:17:29) syzzer: the openvpn protocol is relatively robust against IV reuse, 
but it is still a Bad Idea
(21:17:39) cron2: test running
(21:18:27) syzzer: so, what do you think of my --no-iv "bugfix"?  simple enough 
to include into 2.4?
(21:18:53) cron2: syzer: "20:17 < cron2> test running"
(21:19:37) janjust [~janjust@openvpn/community/support/janjust] è entrato nella 
(21:19:38) syzzer: ah, good, that already includes the deprecation patch?
(21:19:45) krzee: jjk!
(21:19:51) janjust: Evening all, thought I'd join you for a change ;)
(21:19:59) cron2: no, that's just trac 784
(21:19:59) mattock: hi janjust!
(21:20:04) dazo: janjust: welcome!
(21:20:06) cron2: did not see anything else yet
(21:20:09) mattock: so this --no-iv option is odd
(21:20:09) janjust: thx all
(21:20:12) syzzer: not, h,
(21:20:27) mattock: I wonder if the performance benefit for using it is 
actually significant...
(21:20:38) syzzer: gah, typo in address.  will resend
(21:21:18) cron2: I can't even properly test OFB here... as my test server gets 
rebuilt twice a week with mbedTLS which doesn't do that
(21:21:53) janjust: performance?  I'm listening
(21:22:33) syzzer: mattock: not using the IV saves you 16 bytes overhead for CBC
(21:22:38) dazo: janjust: just a side-track discussion if why we have --no-iv 
(21:22:40) syzzer: *AES-CBC
(21:23:03) dazo: s/if/of/
(21:23:06) mattock: janjust: have you tested the performance impact of using 
(21:23:26) mattock: syzzer: for each packet?
(21:23:30) janjust: yeah I tested that some years ago
(21:23:32) syzzer: mattock: yes
(21:23:58) janjust: same as using no auth - it depends on your MTU size; for 
networks with small MTU sizes any bytes saved have a larger impact
(21:24:31) janjust: for default = 1500 bytes you gain on the order of 16 / 1400 
bytes = ~1.5%
(21:24:53) mattock: yeah, so significant, but not incredible improvement
(21:25:16) syzzer: those that want performance should just switch to AES-GCM :)
(21:25:17) janjust: exactly, EXCEPT when you're on older satellite links with 
crooked MTUs of 800 bytes
(21:25:30) dazo: --no-iv have been around since the beginning of our git 
history ... so I think this is just old code we've inherited ... not convinced 
we should have it though
(21:25:43) cron2: syzzer: https://paste.fedoraproject.org/501199/48113873/ - ok?
(21:25:53) mattock: as cron2 tends to say: "we have too many options"
(21:25:58) janjust: --no-iv is a nice debugging feature 
(21:26:12) syzzer: janjust: why is that useful for debugging/
(21:26:25) dazo: (syzzer beat me to it)
(21:26:36) syzzer: cron2: ack
(21:26:44) janjust: if you do it right your traffic will be encrypted exactly 
the same every tyime
(21:26:48) cron2: syzzer: new version, even better I think :-) - 
(21:27:07) syzzer: janjust: that's why we have --cipher none :)
(21:27:32) janjust: syzzer: yes but sometime you want "predictable encryption" 
(21:27:32) cron2: I'm sure --cipher none will crash if you set --no-iv at the 
same time.  Cipher none *loves* to crash on us :-)
(21:27:35) syzzer: cron2: fine too
(21:27:59) syzzer: janjust: for that, just rig the prng ;)
(21:28:48) janjust: true, and I admit that it's an exotic case
(21:29:07) janjust: but the openvpn code path for cipher none is quite 
different compared to having a cipher
(21:29:17) dazo: I can see the academic use of --no-iv and rigged prngs ... but 
I agree with cron2, we have too many options ... and users adding --no-iv into 
production are (hopefulle) close to none, and if they really do need it (MTU < 
800) we will have 2.3 for a bit longer
(21:29:38) ***cron2 ACKs the "deprecate --no-iv" patch
(21:29:40) janjust: OTOH we could also achieve it by specifying an OpenSSL NULL 
(21:29:43) cron2: (that *just* hit the list)
(21:30:52) dazo: feature-ACK on syzzer deprecate --no-iv patch ... need to 
fully test for a full ACK
(21:31:35) syzzer: o/
(21:31:38) syzzer: ar
(21:31:40) syzzer: \o/
(21:31:51) cron2: one-armed crypto bandit?
(21:32:01) dazo: janjust: just a complete side track .... in addition to OSTIF 
funding a security audit of OpenVPN v2.4, PIA have announced they've engaged 
Matt Green to do a security audit on v2.4 as well
(21:32:15) cron2: Wed Dec  7 20:32:05 2016 WARNING: --no-iv is deprecated and 
will be removed in 2.5
(21:32:18) cron2: tested
(21:32:25) janjust: dazo, nice!
(21:32:41) mattock: dazo: and why is PIA working on a separate security audit?
(21:33:00) cron2: syzzer: for 2.5, we should move the "disabling NCP mode 
(--ncp-disable) because not in P2MP ..." line somewhat "down the line" - it's 
printed before anything else usually, which is a bit weird (before the version 
(21:33:15) cron2: I'm not sure who is to blame for that, most likely me
(21:33:32) mattock: Derek from OSTIF gave me some numbers of how much they've 
raised, and the sums are quite substantial already
(21:33:35) dazo: mattock: they were not aware of our OSTIF discussions and had 
approached him before we had our announcement .... Matt Green will have a much 
more crypto focused review though
(21:34:03) mattock: dazo: in any case PIA should work with OSTIF so that the 
reviews don't overlap
(21:34:10) mattock: unnecessarily much at least
(21:34:22) syzzer: yeah, we should just ask OSTIF to focus on the software 
security more than on crypto
(21:34:23) janjust: I agree with mattock; the last thing we'd want is two 
different outcomes
(21:34:37) ***cron2 needs to be afk for ~15 minutes to do christmas cookie 
chocolate covering which needs to be right *now* (while the temperature is just 
(21:34:43) syzzer: as the crypto wonk will focus on the crypto ;)
(21:34:43) dazo: mattock: that's what I've been discussing with caleb1 (on 
#openvpn-devel) and Kelticfox (on PM) today already
(21:34:49) mattock: cron2: have fun!
(21:35:10) mattock: dazo: those guys are from PIA?
(21:35:11) ***janjust fully expects cron2 to fax us all a box of chocolate Xmas 
(21:35:14) dazo: mattock: yes
(21:35:16) mattock: ok
(21:35:20) mattock: good to know
(21:35:32) cron2: you could agree on the mechanics of the reformatting in the 
mean time - how do we get this reviewed properly, without sending "full openvpn 
source" (because the diff will be larger than the original files in most cases) 
across the list...
(21:35:51) syzzer: yeah, I thought a bit about that
(21:36:00) dazo: I think it is good that we have more audits on critical code 
paths ... but the more trivial code paths does not need to be that extensive
(21:37:21) dazo: on the reformatting ... I think we need to do that without ML, 
tbh ... we can push it out to a review branch ... where we can pull it down and 
do git diff -w (--ignore-white-space) to see better the changes
(21:37:56) dazo: the review won't be trivial ... so here we need to put some 
faith into those pushing out these changes and merging them into master
(21:38:14) syzzer: what about (1) define the exact tool, version and flags we 
use for the formatting, (2) get an ACK on that on the list, (3) push a commit 
with just running that on the codebase, (4) get some public ACKs on the commit 
SHA to confirm this is exactly running the tool (5) send style fixup patches 
following the default procedure
(21:38:16) janjust: as long as we don't adopt GNU C coding standards (e.g.   
"if true {" on the same line) then I'll be ok
(21:38:55) syzzer: janjust: we are currently using the GNU style :p
(21:38:58) dazo: janjust: allman style is decided ... spaces (no tabs), width 4 
... always curly braces on if() stuff
(21:39:05) syzzer: and want to switch to allman
(21:39:20) syzzer: https://community.openvpn.net/openvpn/wiki/CodeStyle
(21:39:21) vpnHelper: Title: CodeStyle – OpenVPN Community (at 
(21:39:36) janjust: actually we're inconsisten in style
(21:39:42) dazo: yep!
(21:39:46) syzzer: janjust: yes, but mostly gnu :p
(21:39:49) janjust: +t somewhere; but I really really dislike gnu
(21:39:55) janjust: I remember discussing this in Delft
(21:41:03) dazo: for tool ... I personally found astyle fairly easy to use
(21:41:31) syzzer: dazo: before switching to the tool - what do you think of my 
proposed procedure above?
(21:42:53) dazo: syzzer: sounds reasonable ... step 3) I pressume is 'git push'
(21:43:13) syzzer: sort of 'reproducible reformatting'
(21:43:20) dazo: yes, I like that
(21:43:25) syzzer: yeah, commit & push
(21:43:32) dazo: makes sense
(21:44:16) dazo: should we add a tools/script directory where we have a simple 
script running the indenting properly ... which can be used by patch 
contributors as well?
(21:44:27) janjust: dazo: +!
(21:44:30) janjust: +1
(21:45:53) dazo: :)
(21:46:07) syzzer: yes, something like that.  I was thinking of just putting 
the reformatting flags on the CodeStyle page
(21:46:10) cron2: syzzer: I like that approach (and the "document tool 
invocation in tree")
(21:46:34) cron2: I've started on the CodeStyle page with optiosn for gnu 
indent (does not work really well for me) and uncrustify, but I'm open to use 
astyle if that works
(21:47:12) dazo: I've looked at uncrustify a long time ago ... it is a powerful 
tool ... but it got the same issue OpenVPN have .... too many options
(21:47:33) dazo: (*that* could be an argument why we should use uncrustify 
though ... we're consistent in loving options!)
(21:48:00) dazo: I'm open to use both astyle and uncrustify
(21:48:01) janjust: before you know it cron2 will submit patches to uncrustify 
to remove all sorts of options ;)
(21:48:13) dazo: hehehe
(21:49:09) syzzer: I heven't tried either yet, but as long as it does the job, 
I'm fine with it
(21:49:18) selvanair [~sn...@www.silvermet.ca] è entrato nella stanza.
(21:49:45) janjust: ooo Selva's in Canada.. I always wondered
(21:51:11) selvanair: janjust: logged in from someone else's server.. but 
canada part is right
(21:52:00) janjust: and hi selvanair , of course
(21:52:24) mattock: hi selvanair!
(21:52:28) selvanair: hi guys, looks like I missed the meeting..
(21:52:39) mattock: not really, we still have stuff to discuss
(21:52:52) janjust: and cron2's busy making xmas cookies
(21:53:01) mattock: anyways, cron2 could hide quite a few "remote some rarely 
used options" in midst of "great reformatting"
(21:53:03) mattock: :P
(21:53:06) mattock: remove
(21:53:19) janjust: hehe
(21:53:23) dazo: so ... how do we end up with a decision on astyle vs 
(21:53:35) selvanair: sounds good -- so the Grand Reformattng is on!
(21:53:47) janjust: because we were discussing the procedure and which tool to 
use with what options
(21:53:55) dazo: One part would be that we should have as few clean-ups after 
running the first automated clean-up
(21:54:22) cron2: dazo: since you brought up astyle, you get the job of testing 
it vs. tun.c to see which options are needed to produce a conforming *and* 
useful output :-)
(21:54:52) cron2: tun.c has functions with excessive long argument lists, 
excessive #ifdef orgys, excessively long variable names and declaration lists, 
and all that...
(21:55:03) dazo: fair enough! that's a good test base
(21:55:28) ***syzzer was just playing a bit with astyle
(21:55:47) cron2: my uncrustify options in the wiki page seem to work, at least 
I did not spot anything totally broken anymore.  Ident insisted on reformatting 
stuff that ended up being just ugly afterwards
(21:56:02) cron2: as a side note, my uncrustify suggested options *keep* the 
old function declaration style
(21:56:05) cron2: static void
(21:56:11) cron2: foo(int bar);
(21:56:13) cron2: instead of going for
(21:56:20) cron2: static void foo(int bar)
(21:56:33) cron2: because - especially in tun.c - that caused massive ugliness
(21:56:46) cron2: (remove the ";" up there, declaration, not prototyle)
(21:56:51) syzzer: I usually do prefer the one-line approach, but I can also 
see how that becomes quite ugly with long names...
(21:57:29) selvanair: Looking at the "style manual": isn't it better to break 
lines before operators rather than after it (the so-calle Knuth style becuase 
of the way displayed equations are formatted) -- ignore if its too late to 
discuss style..  
(21:57:54) syzzer: selvanair: no it's not too late
(21:57:57) cron2: init_tun() becomes REALLY ugly if you shift everything to the 
right by 15 characters
(21:58:10) syzzer: I just wrote up some stuff, and it's most definitely open 
for discussion
(21:58:20) selvanair: good to know
(21:58:48) cron2: selvanair: no strong opinion on that.  We should decide 
something and write it up.
(22:00:07) dazo: I generally prefer all on one line as well, but if it makes 
the result unreadable .... readability trumps one line
(22:00:22) selvanair: I think savings = pay + bribe + kickback - rent - beer  
looks better when broken with continuing lines started as '+ kickback' than 
leaving the + in the prvious line..
(22:01:11) selvanair: dazo: thought we are imposing 80 char per line max..
(22:01:33) dazo: yeah .... I'd prefer 100, tbh
(22:01:53) selvanair: me too
(22:01:58) dazo: (who does development on a text terminal fixed at 80 chars 
these days?)
(22:02:03) ***cron2 
(22:02:16) cron2: lines longer than 80 characters are usually a sign of some 
(22:02:17) dazo: but you don't have to be fixed at 80?
(22:02:18) mattock: is the terminal green?
(22:02:23) dazo: lol
(22:02:25) selvanair: Not even me who runs vi as init :)
(22:02:27) syzzer: dazo: well, I like the linux-argument - too much 
indentation?  just add functions.
(22:02:28) janjust: you've still got a teletype terminal, cron2?
(22:02:31) cron2: mattock: you know how my terminals look like :)
(22:02:43) cron2: janjust: I have an original VT220 in my basement, yes.
(22:02:54) dazo: syzzer: I agree ... but they even have tab-with 8
(22:02:59) janjust: no I meant teletype: the one where you type a line and it 
is printed on paper instead of on screen
(22:03:14) janjust: the original VT220 was/is awesome :)
(22:03:15) cron2: never had one of those... I'm one of the young ones!
(22:03:21) dazo: :-P
(22:03:22) syzzer: dazo: so with our indent with of 4, we are quite mild ;)
(22:03:45) ***janjust is old but not gray/grey
(22:03:50) mattock: so do we have cases where lines go way past 80 characters?
(22:03:50) dazo: yeah, but still we have so much code which do have a bit too 
much nesting already ....
(22:04:09) syzzer: mostly function call with lots of arguments, or debug strings
(22:04:16) cron2: we do, but everything syzzer has touched has been wrappified
(22:04:22) dazo: I have a few patches recently which I had to reformat .... 
they went up to 90-ish
(22:05:10) ***syzzer has been slapping people with the 80 char limit, hehe
(22:05:27) mattock: so would a limit of 100 characters simplify the Great 
Reformatting significantly?
(22:05:43) syzzer: hm, I don't think so actually
(22:05:55) cron2: no... most of it is braces, proper indentation, and breaking 
150+ lines
(22:06:41) syzzer: (I do like Selva's suggestion wrt the Knuth style btw)
(22:06:49) cron2: write it up :)
(22:07:01) syzzer: selvanair: will you do the honours?
(22:07:08) selvanair: syzzer: I edited the code-style page -- please revert if 
you dont like it
(22:07:13) cron2: that was quick
(22:07:26) syzzer: hehe, amazing
(22:07:26) selvanair: he he -- a bot working in parallel
(22:07:56) mattock: do we have git pre-push hooks or something that warn about 
wrong style (e.g. tabs)?
(22:08:27) syzzer: selvanair: works for me!
(22:08:28) cron2: not yet
(22:08:50) syzzer: does github support hooks?
(22:09:16) syzzer: or you want local hooks?
(22:09:20) dazo: the pre-push stuff needs to be installed locally ... and can't 
easily be done "automatically" upon git clone
(22:09:40) cron2: I would need local hooks, otherwise it's too late if github 
refuses something while gitlab and sf take it
(22:09:43) dazo: otherwise we need something on the git remotes
(22:09:51) dazo: yes
(22:10:21) syzzer: ah, ok, mostly for the core devs then
(22:10:39) dazo: yeah
(22:10:42) ***syzzer is admin at work, and likes writing server-side hooks to 
slap colleagues ;)
(22:11:03) dazo: should we have padding around operators?
(22:11:10) dazo:  a=bar((b-c)*a,d--);
(22:11:11) dazo: vs
(22:11:15) dazo:     a = bar((b - c) * a, d--);
(22:11:37) syzzer: after a , for sure
(22:11:49) dazo: agreed
(22:11:55) cron2: "sort of, but not enforced" - for long operands, I like 
spaces, but for "b+3" I find it hard to read
(22:12:31) cron2: around " = ", yes, in all cases
(22:12:35) dazo: if we want to have some kind of automated stuff, we need to 
have some consistency
(22:12:40) cron2: after a ", " - in most cases, maybe even "all cases"
(22:12:57) selvanair: and  around " > " 
(22:13:00) cron2: dazo: uncrustify can be told to leave things alone
(22:13:10) syzzer: yeah, this is also one case where I'd say "it depends"
(22:13:22) cron2: "add, remove, ignore, warn" is typically the value you can 
set an "insert_space_for_xyz" to
(22:13:45) dazo: well, "it depends" is quite vague for a styling tool :)
(22:14:07) syzzer: if it is one or the other, I'd choose spaces
(22:14:34) dazo: we can also do "don't touch, let it be"
(22:14:35) syzzer: not even sure if I prefer 'spaces' or 'do not touch'
(22:14:47) cron2: I find the example *with* spaces not exactly clear either... 
and might want to write a = bar( (b-c) * a, d-- )  instead
(22:15:21) cron2: so I'd go for "we document our preferences in the CodingStyle 
page, and tell the tool to leave it alone"
(22:15:30) cron2: so, next round of cookie massacre...
(22:15:31) syzzer: fine bt me
(22:15:57) mattock: done with code style?
(22:16:05) dazo: char &foo1;
(22:16:09) syzzer: mattock: never!
(22:16:10) dazo: char& foo1;
(22:16:16) syzzer: char &foo
(22:16:22) syzzer: just like char *foo
(22:16:24) dazo: agreed :)
(22:16:29) janjust: char& foo1 reminds me of c++ too much
(22:17:03) cron2: +1
(22:17:12) cron2: (cookie massacre needs more cooling down first)
(22:17:30) ***janjust wants a cookie
(22:18:07) cron2: no 3D scanner here yet :(
(22:19:56) syzzer: example updated with "const char *"
(22:20:38) cron2: no blank before the opening bracket in functions?
(22:20:50) cron2: something_equals(dummy) vs. something_equals (dummy)
(22:21:00) dazo: Heres another one ....
(22:21:01) dazo: --------------
(22:21:02) syzzer: I'd say not
(22:21:02) janjust: or something_equals( dummy )
(22:21:03) dazo: isFoo = true;
(22:21:04) dazo: if (isFoo) {
(22:21:04) dazo:     bar();
(22:21:04) dazo: } else {
(22:21:04) dazo:     anotherBar();
(22:21:05) dazo: }
(22:21:07) dazo: isBar = false;
(22:21:09) dazo: ------------------
(22:21:11) dazo: vs
(22:21:13) dazo: ---------------
(22:21:20) dazo: isFoo = true;
(22:21:25) dazo:    // new line
(22:21:26) cron2: dazo: that's easy: braces get their own line, always
(22:21:30) dazo: if (isFoo) {
(22:21:30) dazo:     bar();
(22:21:30) dazo: } else {
(22:21:30) dazo:     anotherBar();
(22:21:31) dazo: }
(22:21:32) cron2: and no C++ comments
(22:21:35) dazo:    // new line
(22:21:35) mattock: (and it goes on) :P
(22:21:38) dazo: isBar = false;
(22:21:41) dazo: ---------------
(22:21:57) cron2: covered by the myfunction() example :)
(22:21:58) dazo: so should it be compact, or have a blank line before/after an 
if() block?
(22:22:20) dazo: so compact
(22:22:21) janjust: depends on the context/code,dazo
(22:22:22) cron2: your if() block is not right
(22:22:29) cron2: :)
(22:22:33) syzzer: yeah, compact
(22:22:33) janjust: mostly compact , but sometimes a newline "just makes sense"
(22:22:37) dazo: well ... irc messed it up
(22:22:41) syzzer: exactly, like prose ;)
(22:22:52) cron2: no, you use the } else { thing we already decided against
(22:23:06) selvanair: no blank after functions is ok, but after if its needed 
isn't it: if(x) is ugly -- I'd prefer use "if (x)" 
(22:23:12) dazo: ahh, ignore that ... that was not the crux here :)
(22:23:12) cron2: and "what janjust says", this is a leave-alone thing
(22:23:35) cron2: if ( and for ( etc. needs the blank, yes
(22:23:52) syzzer: selvanair++
(22:24:01) cron2: very short if (...) blocks do not warrant an empty line, more 
complex stuff does
(22:24:04) syzzer: (as does the example)
(22:24:12) dazo: https://paste.fedoraproject.org/501248/14811422/
(22:24:27) dazo: tun.c ... with astyle --style=allman --indent=spaces=4 
--convert-tabs --max-code-length=80 --align-reference=name --lineend=linux 
--indent-switches  -j
(22:24:54) cron2: we do not want --convert-tabs
(22:25:12) syzzer: cron2: we don't?
(22:25:19) dazo: oh yes we do
(22:25:32) dazo: we don't want a mixture of tab/spaces when we've decided for 
spaces only
(22:25:39) cron2: I'm fairly sure we agreed that we keep tabs in front of the 
lines, with tabstop=8
(22:25:50) dazo: nope,  only spaces is what I recall
(22:25:59) cron2: you recall wishful thinking :-)
(22:26:14) syzzer: I recall that too
(22:26:18) janjust: mmm looks pretty good, dazo, but what about function 
declarations with multiple parms? check out  check_subnet_conflict vs 
(22:26:26) dazo: tabs vs. spaces: "not mixed"
(22:26:27) dazo:     majority for "only spaces, no tabs"
(22:26:27) dazo:     this is what we change to: all spaces 
(22:26:31) dazo: http://community.openvpn.net/openvpn/wiki/MunichHackathon2014
(22:26:32) vpnHelper: Title: MunichHackathon2014 – OpenVPN Community (at 
(22:26:38) selvanair: me too -- spaces only please
(22:26:45) janjust: +1
(22:26:45) mattock: damn, he found documentation
(22:26:46) janjust: +
(22:26:49) janjust: +1
(22:26:49) selvanair: ++1
(22:26:54) janjust: 1++
(22:26:56) selvanair: ++2
(22:27:04) syzzer:   -
(22:27:06) cron2: I was looking at the page, but couldn't find the paragraph 
yet, so, well, since we've agreed...
(22:27:29) cron2: btw, it has some say about line length...
(22:27:55) dazo: right ... so syzzer tricked us to 80 charsd!
(22:28:05) dazo: s/sd$/s/
(22:28:12) syzzer: well, "what we have now" is 80, just not enforced :p
(22:28:20) dazo: hehe :)
(22:28:32) cron2: looking at how we spent the last 60 minutes, I think we 
should focus on "reindent, tabs-to-spaces, add braces to single-line if/else 
terms, and do not bikeshed the conversion to death"
(22:28:42) janjust: Repeat: what about function declarations with multiple 
parms? check out  check_subnet_conflict vs ifconfig_options_string
(22:28:47) mattock: "syzzer voluntereers to make the non-consistent files 
consistent "
(22:29:02) syzzer: oh, damn :')
(22:29:22) dazo: #if 0 /* too many false positives */
(22:29:24) mattock: +1 on no more bikeshedding
(22:29:25) cron2: janjust: I'd go for "wrap depending on total length and try 
to make it non-ugly"
(22:29:40) janjust: hehe cron2, and how do we teach the tool to do that?
(22:29:42) syzzer: cron2++
(22:29:48) cron2: janjust: we don't
(22:30:06) dazo: my pastebin ... tries to do some 80 chars limitations
(22:30:06) cron2: "every option gets its own line" can be about as ugly as "all 
crammed in a single large line"
(22:30:20) cron2: half the annoyance of tun.c is argv_...() calls with "every 
argument on a single line"
(22:30:24) dazo: ack ... that's a case-by-case too
(22:30:40) selvanair: As for a conversion of exisitng code,  a reasonable 
output is good enough -- we can impose style more strongly for new code. 
(22:30:41) dazo: (but astyle shouldn't touch that with my args)
(22:30:42) cron2: and then 3 of those, 20 lines of code for 3 function calls 
and 2 if()s
(22:30:50) cron2: selvanair: +1
(22:31:31) cron2: dazo: the indentation of the function prototypes looks ugly - 
they should be aligned with the first argument, not "with something arbitrary", 
(22:31:41) cron2: (line 66-73)
(22:31:42) dazo: which line?
(22:31:56) dazo: it's aligned isn't it?
(22:32:11) selvanair: Thngs like msg(M_INFO, "loong text....") is going to be 
hard to get automatically wrapped into 80 cahrs, but that's ok isn't it?
(22:32:18) dazo: oh you mean the argument names? 
(22:32:20) cron2: well, in the pastebin, it's not, but that might be variable 
font length
(22:32:45) cron2: I see the first set as aligned to the "i" of 
"netsh_ifconfig", and the second set to the "d" of "netsh_set_dns6_servers", 
which is different from each other, and makes no sense
(22:32:55) syzzer: variable font length?  verbatim here
(22:33:13) cron2: syzzer: where are the parameters aligned to for you?
(22:33:17) syzzer: yes
(22:33:28) dazo: selvanair: yes, agreed on long string lines
(22:33:29) syzzer: right under the ( +1
(22:33:31) dazo: cron2: http://imgur.com/a/LdQlB
(22:33:32) vpnHelper: Title: Imgur: The most awesome images on the Internet (at 
(22:33:42) cron2: *sigh*
(22:33:50) syzzer: yes, that
(22:33:58) cron2: chrome stinks.  firefox shows them nicely aligned 
(22:34:07) dazo: :)
(22:34:13) selvanair: lines 66-73 look ok on my browser..
(22:34:21) selvanair: firefox++
(22:34:22) cron2: the internet is so full on broken software
(22:34:29) dazo: lol :)
(22:34:50) cron2: generally speaking, firefox sucks more often than chrome, 
though (like in sucking up more memory than EMACS ever did...)
(22:35:35) dazo: that's true .... firefox is getting where netscape once was 
... where firefox was a relief and incredibly fast compared to netscape
(22:35:45) cron2: do we have agreement on the indent level for switch/case?
(22:36:05) cron2: I tend to indent only *two* for the "case" labels, and two 
more for the code block (so code is indented +4, not +8)?
(22:36:09) cron2: line 273
(22:36:13) syzzer: dazo: I still see ifs without braces int he paste, btw
(22:36:26) dazo: oh ... which line?
(22:36:39) syzzer: 380
(22:36:41) cron2: syzzer: interesting.  The ones I saw have been converted 
(which are the ones I remember from my own tests)
(22:36:51) dazo: eek
(22:36:56) cron2: indeed
(22:37:13) selvanair: and more at line 986
(22:37:24) dazo: -j should have taken care of that, I though ... need to double 
(22:37:34) syzzer: re switch case:  either 4-4 or 0-4
(22:37:48) syzzer: 2-2 would be confusing I think, given that we do 4 for 
(22:38:27) selvanair: 4-4
(22:38:27) dazo: syzzer++
(22:38:39) syzzer: I'd say 4-4 too
(22:38:42) cron2: well, with 2-2, you end up with +4 for the code level, and 
the label gets a "relative -2" so is easy to spot
(22:38:54) cron2: 4-4 gives you +8 for the code level, which quickly adds up
(22:39:19) cron2: (interesting enough, uncrustify got that one wrong with my 
sample options and gave me 2-4, which looks weird)
(22:39:37) dazo: I don't see an issue with that ... as if that gets too much, 
the code should be refactored
(22:39:40) cron2: but anyway, if everyone wants 4-4, so be it
(22:39:51) syzzer: cron2: that will be tricky to teach to editors though...
(22:40:24) cron2: syzzer: what, too lazy to do your indents by hand?  You have 
no sense for beauty!
(22:40:45) syzzer: cron2: exactly!
(22:41:02) janjust: hmm if this indentation topic is continuing until 23:00 
then we're going to see a Godwin ...
(22:41:03) selvanair: +1 for indent by hand and mess it up camp..
(22:41:05) cron2: dazo: well, yes.  There's 71876 lines of .c that need 
refactoring in src/openvpn/...  but that's not a strong argument here an dnow
(22:41:27) cron2: selvanair: of course not "mess it up".  Do it properly, with 
loving care! :-)
(22:41:46) selvanair: more out of pride than love...
(22:42:09) cron2: anyway, the pastebin from dazo needs fixing the if(), but 
otherwise it looks like something far better than today
(22:42:38) cron2: so if it learns to add the missing braces, we're at 
"worksforme" there
(22:42:44) syzzer: If we have a consistent code style, we code even consider 
adding the vim auto formatting thingies into our c files
(22:42:46) dazo: absolutely ... if we can't get astyle doing if() properly, 
then it's most likely uncrustify we'll end up with
(22:43:03) cron2: stuff like line 468 will have to be done manually when the 
great tun.c refactoring starts
(22:43:07) mattock: janjust: I suspended a while back already :P
(22:43:24) selvanair: quite impressive actually -- far from the old days of 
running indent
(22:43:49) mattock: so topic #3 anyone?
(22:43:58) cron2: dazo: but why is it adding if()s to the stuff in the 
(22:44:06) cron2: well, braces to the if()s...
(22:44:14) dazo: I dunno ... trying to figure that out
(22:44:29) cron2: mattock: what selva suggests makes sense :)
(22:44:44) mattock: cron2: on GitHub?
(22:44:54) cron2: iservice needs to be installed if [X] gui is checked, so 
maybe make that "[X] GUI and interactive service"
(22:45:13) cron2: mattock: can't remember if that was on the list or on github, 
but where the discussion was had
(22:45:47) syzzer: on the list, and I agree
(22:46:06) selvanair: I'm proposing to install iservice always with the core. 
Then GUI and TAP and possibly openvpnserv2 are all independent add ons
(22:46:10) syzzer: at least the discussion I saw
(22:46:32) mattock: the GitHub link contains a lot more refactorings
(22:47:12) mattock: we already have a plan, but the question is/was: who large 
changes are we willing to accept to the Windows installer beyond 2.4_rc?
(22:47:12) dazo: selvanair++
(22:47:17) cron2: mmmh, yeah - people *might* want to script openvpn with 
(22:47:35) selvanair: cron2: exactly
(22:47:38) cron2: mattock: "anything that makes sense", and "not having 
iservice if not installing openvpnserv2" doesn't, so "bugfix"
(22:47:44) selvanair: Also reduces dependencies.
(22:48:00) selvanair: Comamndline install still will have the full enchilada of 
(22:48:25) dazo: which makes sense ... proper advanced users will use the 
command line primarily and not a gui wizard
(22:48:59) selvanair: And be responsible choosing right options
(22:50:08) selvanair: mattock: as long as we are not taking anything out of the 
default installation, there are no big changes. 
(22:50:10) dazo: yes
(22:51:31) mattock: ok, fine by me
(22:52:01) mattock: we just need to refactor the installer carefully in order 
not to introduce bugs in installers that users would not expect to change much
(22:52:19) mattock: fortunately the refactoring should make things quite a bit 
(22:52:34) mattock: anything else for today?
(22:53:10) cron2: timeline for reformatting up to rc2
(22:53:20) cron2: and who does it
(22:53:42) syzzer: well, you or dazo, since you are the ones allowed to push to 
(22:53:58) syzzer: that is the 'run the script, push to master' bit
(22:54:08) selvanair: mattock: sure: we do not change any sections, but just 
hide some sections and make it harder to unselect, always install openvpn, 
iservice and dlls -- no point in making dlls un-selectable.
(22:54:22) dazo: mattock: as long as we have the sane defaults _we_ recommend 
users to use, and don't add too many whistles and bells (only those options 
which are truly needed, not any "nice to have"-option) ... then I think we're 
on a good path, and the possbilities to fail are less
(22:54:29) cron2: syzzer: you have point there :-)
(22:54:32) syzzer: I'll make some time to scavenge through files afterwards, 
looking for ugliness to fix
(22:54:42) dazo: great
(22:55:38) syzzer: maybe push to a branch first, so people can verify the 
'reproducible reformatting'
(22:55:42) dazo: cron2: regarding the if() braces ... for some reason it 
currently doesn't add {} to if(), if it is some kind of if() nesting ... not 
sure if it's a bug or feature ... looking through options once again
(22:56:00) cron2: ok.  dazo said he has some time tonight - so if we could get 
a mail to -devel with "this is the tool we intend to use, you have one day to 
scream!" by tomorrow that should get us started
(22:56:02) dazo: syzzer: that's my thought as well ... a reformat branch
(22:56:07) mattock: dazo, selvanair: agreed
(22:56:25) mattock: just wanted a consensus on this, because changes in the 
Windows installer affect tons of users
(22:56:40) cron2: then "run the script with the options suggested" and push to 
"reformat", people ACK that this is really only "what the tool does", and then 
merge that
(22:57:02) dazo: mattock: which is why simplicity is key ... most users windows 
don't understand lots of these options anyhow
(22:57:15) dazo: cron2++
(22:57:44) syzzer: agreed
(22:58:03) selvanair: mattock: assuming most of those tons of users all install 
using defaults, we can make sure they are not affected.
(22:59:25) dazo: selvanair: mattock: we will do some mistakes when reducing 
options and providing, in our view, saner options .... so we will most likely 
need to run a few installers and tweak the defaults for the GUI install .... 
but that shouldn't stop us from trying
(23:00:39) mattock: selvanair: we can be pretty sure 99%+ are using the 
defaults - as you pointed out, SecService handles setting up config and log dirs
(23:00:51) mattock: and nobody (afaik) has complained about "where is my log 
(23:00:58) selvanair: yes, tests are still needed. I test on a couple of 
machines before submitting any patches and mattock is also very heavily 
invested in testing.. I think we can finalize it by early next week.
(23:01:00) mattock: =nobody checks of the Service section
(23:01:20) mattock: it would be good to get the installer changes into rc2
(23:01:25) mattock: rather than in 2.4.0
(23:02:00) mattock: let's merge what we have available now and apply more 
fixes/refactoring on top of that
(23:02:15) selvanair: mattock: first figure out what to do with active-setup (I 
think its good now :) and net40. Then we can do this quick.
(23:02:25) mattock: +1
(23:02:25) selvanair: mattock: agreed
(23:02:55) mattock: I will work on this stuff tomorrow and on Friday
(23:03:33) mattock: everything done for today, then?
(23:03:40) mattock: getting rather late here
(23:03:52) selvanair: 4pm here
(23:04:09) janjust: toronto?
(23:04:12) selvanair: yep
(23:04:23) syzzer: I think we're good for today, yes
(23:04:25) janjust: I saw some nice vids about the snow&ice in toronto
(23:04:34) krzee: i liked toronto
(23:04:39) plaisthos: toronto is nice
(23:04:48) janjust: next openvpn meeting is in toronto!
(23:04:53) krzee: woooo
(23:04:58) mattock: that would be something!
(23:05:01) krzee: i can actually make it easier!
(23:05:06) selvanair: hehe -- welcome..
(23:05:16) cron2: did I send the mail?
(23:05:58) janjust: how, krzee ?
(23:06:08) janjust: ow never mind, I understand your remark
(23:06:10) janjust: it IS getting late
(23:06:11) krzee: janjust: much easier for me to take it to toronto
(23:06:22) krzee: it wont make anything easier for anybody else tho :D
(23:06:31) krzee: make it to toronto*
(23:06:42) ***janjust has never been to toronto. Vancouver is nice tho
(23:07:06) ***syzzer neither, but would like to :)
(23:07:13) janjust: we can all get on the OpenVPN Inc corporate jet and fly 
straight to Toronto
(23:07:23) mattock: I'm sure :P
(23:07:27) cron2: I'm in!
(23:07:39) plaisthos: :)
(23:08:02) mattock: and here ends the official meeting chatlog :)
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

Reply via email to