[Openvpn-devel] [PATCH applied] Re: Ignore SIGUSR1/SIGHUP during exit notification

2016-06-07 Thread Gert Doering
ACK. Stared at the code, and managed to reproduce the race by pressing ctrl-c at exactly the right spot (shortly before the "pull-filter reject" would trigger a SIGUSR1) and saw my ctrl-c being ignored - with the patch, things work as expected: Tue Jun 7 23:00:08 2016 us=714409 Ignoring SIGUSR1

Re: [Openvpn-devel] [PATCH 2/2] Save exit-notify timer during a restart

2016-06-07 Thread Gert Doering
Hi, On Sun, Jun 05, 2016 at 05:41:24PM -0400, Selva Nair wrote: > When exit-notify is specified, SIGTERM could be lost if a SIGUSR1/SIGHUP > restart happens during the exit notification interval. This is > particularly apparent when pull-filter reject causes a repeated SIGUSR1 > restart cycle. >

[Openvpn-devel] [PATCH applied] Re: Add an option to filter options received from server

2016-06-07 Thread Gert Doering
ACK. Stared at the code, tested, works. I like the way using a linked list actually made this code more simple (which hints that the previous version might not have been ideal, but anyway). I'm not sure I agree with the style used in apply_pull_filter() - I might have coded it as "first do the

Re: [Openvpn-devel] potential segfault on strlen(NULL)

2016-06-07 Thread Илья Шипицин
2016-06-07 19:11 GMT+05:00 Gert Doering : > Hi, > > On Tue, Jun 07, 2016 at 06:04:54PM +0500, ?? wrote: > > as I see, there's call to format_hex_ex with separator=NULL here: > > Interesting find. > > This code is funny - format_hex_ex() is called from various places with > sep

Re: [Openvpn-devel] Valgring findings

2016-06-07 Thread Selva Nair
On Tue, Jun 7, 2016 at 10:41 AM, Gert Doering wrote: > On Tue, Jun 07, 2016 at 05:33:07PM +0300, Samuli Seppänen wrote: > > > I played with valgrind a bit > > > > > > https://travis-ci.org/chipitsine/openvpn/jobs/135869065 > > > > > > Looks like there are leaks in openssl code, should we suppress

Re: [Openvpn-devel] Valgring findings

2016-06-07 Thread Илья Шипицин
I thought to run valgrind separately, but it might be interesting to run it under "make check" as well 2016-06-07 19:33 GMT+05:00 Samuli Seppänen : > > Hello, >> >> I played with valgrind a bit >> >> https://travis-ci.org/chipitsine/openvpn/jobs/135869065 >> >> Looks like there are leaks in opens

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Selva Nair
On Tue, Jun 7, 2016 at 6:31 AM, Samuli Seppänen wrote: > > Given that we do not have plenty of developer time around, we should > focus > > *development* time on git master / 2.4 - and that includes adding testing > > frameworks. > > +1. We should focus on doing what it takes to (finally) get 2.4

Re: [Openvpn-devel] [PATCH 1/1] Ignore SIGUSR1/SIGHUP during exit notification

2016-06-07 Thread Selva Nair
On Tue, Jun 7, 2016 at 3:06 AM, Gert Doering wrote: > Hi, > > On Tue, Jun 07, 2016 at 12:44:20AM -0400, Selva Nair wrote: > > This allows exit notification to complete and finally trigger SIGTERM. > > The current practice of allowing a restart in this state clears > > the exit notification timer

Re: [Openvpn-devel] Valgring findings

2016-06-07 Thread Gert Doering
Hi, On Tue, Jun 07, 2016 at 05:33:07PM +0300, Samuli Seppänen wrote: > > I played with valgrind a bit > > > > https://travis-ci.org/chipitsine/openvpn/jobs/135869065 > > > > Looks like there are leaks in openssl code, should we suppress it? > > I can't comment on the leaks themselves, but I wonde

Re: [Openvpn-devel] Valgring findings

2016-06-07 Thread Samuli Seppänen
Hello, I played with valgrind a bit https://travis-ci.org/chipitsine/openvpn/jobs/135869065 Looks like there are leaks in openssl code, should we suppress it? I can't comment on the leaks themselves, but I wonder if it would make sense to run OpenVPN under Valgrind in "make check". For exa

Re: [Openvpn-devel] potential segfault on strlen(NULL)

2016-06-07 Thread Gert Doering
Hi, On Tue, Jun 07, 2016 at 06:04:54PM +0500, ?? wrote: > as I see, there's call to format_hex_ex with separator=NULL here: Interesting find. This code is funny - format_hex_ex() is called from various places with separator=NULL, and has been that way since at least 2005.

[Openvpn-devel] Valgring findings

2016-06-07 Thread Илья Шипицин
Hello, I played with valgrind a bit https://travis-ci.org/chipitsine/openvpn/jobs/135869065 Looks like there are leaks in openssl code, should we suppress it? Cheers, Ilya Shipitsin

[Openvpn-devel] potential segfault on strlen(NULL)

2016-06-07 Thread Илья Шипицин
Hello, I'm investigating some cppcheck findings, for example: [src/openvpn/buffer.c:442] -> [src/openvpn/buffer.c:447]: (warning) Either the condition 'if(separator&&i&&!(i%(space_break_flags&255)))' is redundant or there is possible null pointer dereference: separator. [src/openvpn/buffer.c:443]

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Samuli Seppänen
hi, On Tue, Jun 07, 2016 at 01:23:55PM +0500, ?? wrote: mingw is an official way of building windows packages. I guess something like that appliable for ARM as well (I haven't heard about compilers running on those machines) $ uname -mo armv5tel GNU/Linux $ gcc -v Using

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Samuli Seppänen
It seems that a summary of how Vagrant operates is in order here. Vagrant uses pre-built images as a starting point. These images do not (and should not) be built by OpenVPN developers. The only things _we_ have to maintain are the Vagrant files, which are basically recipies for configuring the

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Samuli Seppänen
Given that we do not have plenty of developer time around, we should focus *development* time on git master / 2.4 - and that includes adding testing frameworks. +1. We should focus on doing what it takes to (finally) get 2.4 out of the door. Fairly soon afterwards we can stop worrying about 2.3

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Gert Doering
hi, On Tue, Jun 07, 2016 at 01:23:55PM +0500, ?? wrote: > mingw is an official way of building windows packages. I guess something > like that appliable for ARM as well (I haven't heard about compilers > running on those machines) $ uname -mo armv5tel GNU/Linux $ gcc -v Using

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Gert Doering
Hi, On Tue, Jun 07, 2016 at 01:27:52PM +0500, ?? wrote: > as for Travis-CI builds, there's such possibility already > > 1) register at github.com > 2) add your own repo to travis-ci.org > 3) voila, you can attack Pentagon from travis-ci cloud > > the way of "add some attacki

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Илья Шипицин
oops, I was wrong. 2016-06-07 13:28 GMT+05:00 Arne Schwabe : > Am 07.06.16 um 10:23 schrieb Илья Шипицин: > > mingw is an official way of building windows packages. I guess something > > like that appliable for ARM as well (I haven't heard about compilers > > running on those machines) > > > > Ra

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Arne Schwabe
Am 07.06.16 um 10:23 schrieb Илья Шипицин: > mingw is an official way of building windows packages. I guess something > like that appliable for ARM as well (I haven't heard about compilers > running on those machines) > Raspberry pi! Arne

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Илья Шипицин
as for Travis-CI builds, there's such possibility already 1) register at github.com 2) add your own repo to travis-ci.org 3) voila, you can attack Pentagon from travis-ci cloud the way of "add some attacking code to openvpn codebase" seems to be much more complicated 2016-06-07 13:20 GMT+05:00 G

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Илья Шипицин
mingw is an official way of building windows packages. I guess something like that appliable for ARM as well (I haven't heard about compilers running on those machines) so, if we can catch an issue during such compile, it is good. 2016-06-07 12:58 GMT+05:00 Samuli Seppänen : > I stand corrected

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Gert Doering
Hi, On Tue, Jun 07, 2016 at 10:58:42AM +0300, Samuli Seppänen wrote: > I stand corrected. However, cross-building is not a replacement for > building on the actual OS. > > Do cross-builds generally catch useful issues, or do they tend to catch > issues related to the cross-building environment

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Gert Doering
Hi, On Tue, Jun 07, 2016 at 11:08:47AM +0300, Samuli Seppänen wrote: > > we can't open this to the world, as the t_client tests need sudo > > privileges, so anyone who can push a patch to a testing tree can run > > arbitrary commands on the buildslaves ("just build whatever you want > > into somet

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Gert Doering
Hi, On Mon, Jun 06, 2016 at 10:39:12PM +0200, David Sommerseth wrote: > > On top of that, adding unit tests into an existing code like openvpn > > will involve a lot of refactoring. The ROI on backporting any such tests > > to 2.3 does not look worth the effort. > > Yes, lots of the code may nee

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Samuli Seppänen
Hi, On Tue, Jun 07, 2016 at 10:30:57AM +0300, Samuli Seppänen wrote: This is probably correct: the codebase is complex enough to cause breakage on many types of changes, no matter how carefully the code is reviewed. This is often because of the sheer number of options and their invisible inter

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Samuli Seppänen
I stand corrected. However, cross-building is not a replacement for building on the actual OS. Do cross-builds generally catch useful issues, or do they tend to catch issues related to the cross-building environment itself? -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc f

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Gert Doering
Hi, On Tue, Jun 07, 2016 at 10:30:57AM +0300, Samuli Seppänen wrote: > This is probably correct: the codebase is complex enough to cause > breakage on many types of changes, no matter how carefully the code is > reviewed. This is often because of the sheer number of options and their > invisibl

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Илья Шипицин
it is not true that Travis-CI is limited to Linux/Ubuntu, at least there's Mac OS X. and we can set up (later) cross builds for MIPS/ARM/Windows/whatever (not sure about "make check") cross build would be good starting point, if there was such thing already, we could notice that mingw build got br

Re: [Openvpn-devel] [PATCH] Another fix related to unit test framework

2016-06-07 Thread Samuli Seppänen
… IMO, the unit testing patches shouldn't have been merged into the release branch I agree. This patch was in retrospective clearly not ready for a release branch. A lot of people spend time to hot fix a broken build. My root cause analysis boils down to: Developers cannot detect multi-

Re: [Openvpn-devel] [PATCH 1/1] Ignore SIGUSR1/SIGHUP during exit notification

2016-06-07 Thread Gert Doering
Hi, On Tue, Jun 07, 2016 at 12:44:20AM -0400, Selva Nair wrote: > This allows exit notification to complete and finally trigger SIGTERM. > The current practice of allowing a restart in this state clears > the exit notification timer data and thus loses the SIGTERM. This is along the lines I was t

[Openvpn-devel] [PATCH 1/1] Ignore SIGUSR1/SIGHUP during exit notification

2016-06-07 Thread Selva Nair
This allows exit notification to complete and finally trigger SIGTERM. The current practice of allowing a restart in this state clears the exit notification timer data and thus loses the SIGTERM. Trac #687 Signed-off-by: Selva Nair --- src/openvpn/sig.c | 25 - 1 file

[Openvpn-devel] [PATCH 0/1] Another take on preserving SIGTERM

2016-06-07 Thread Selva Nair
Hi, Here is an alternative to my previous patch addressing the same point: "Save exit-notify timer during a restart" (http://article.gmane.org/gmane.network.openvpn.devel/11807) This one (patch in next email) may be a more palatable approach. Selva Selva Nair (1): Ignore SIGUSR1/SIGHUP duri