Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2018-02-02 Thread Samuli Seppänen
Il 02/02/2018 03:00, Simon Rozman ha scritto: > Hi, > >>> I have tested and am confirming MSVC is happy with the block_dns.c now. >>> >>> Should your patch be merged, I shall rebase the "[PATCH 09/13] >> Signed/unsigned warnings of MSVC resolved" to the new master and deliver >> the next version.

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2018-02-01 Thread Simon Rozman
Hi, > > I have tested and am confirming MSVC is happy with the block_dns.c now. > > > > Should your patch be merged, I shall rebase the "[PATCH 09/13] > Signed/unsigned warnings of MSVC resolved" to the new master and deliver > the next version. > > I finally came around to merging the get_interf

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2018-01-25 Thread Gert Doering
Hi, On Wed, Dec 06, 2017 at 05:10:45PM +, Simon Rozman wrote: > > > The get_interface_metric() function should get a more thorough rewrite > > than just a compiler warning shut-up. So the patch will probably get divided > > in two - the simple signed/unsigned fixes and get_interface_metric() >

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-09 Thread Selva Nair
Hi, On Thu, Dec 7, 2017 at 12:32 PM, Gisle Vanem wrote: > Simon Rozman wrote: > >> However, I did stare-review your code: >> - It does not introduce any new Windows API calls it has not used before. >> - It compiles fine. > > It also builds fine here with cl v19.11. > But using clang-cl v5, I'm g

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-07 Thread Gisle Vanem
Simon Rozman wrote: However, I did stare-review your code: - It does not introduce any new Windows API calls it has not used before. - It compiles fine. It also builds fine here with cl v19.11. But using clang-cl v5, I'm getting: In file included from src/openvpn/argv.c:36: ./src/openvpn/sy

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-06 Thread Gert Doering
Hi, On Thu, Dec 07, 2017 at 06:41:03AM +, Simon Rozman wrote: > > > Should your patch be merged, I shall rebase the "[PATCH 09/13] > > Signed/unsigned warnings of MSVC resolved" to the new master and deliver > > the next version. > > > > Yes, if you can review and ack/nak it :) > > You mean G

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-06 Thread Simon Rozman
Hi, > > Should your patch be merged, I shall rebase the "[PATCH 09/13] > Signed/unsigned warnings of MSVC resolved" to the new master and deliver > the next version. > > Yes, if you can review and ack/nak it :) You mean Gert to ack/nak it? I don't believe I have earned enough reputation to do it

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-06 Thread Selva Nair
Hi, On Wed, Dec 6, 2017 at 12:10 PM, Simon Rozman wrote: > Hi, > >> > The get_interface_metric() function should get a more thorough rewrite >> than just a compiler warning shut-up. So the patch will probably get divided >> in two - the simple signed/unsigned fixes and get_interface_metric() >> r

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-06 Thread Simon Rozman
Hi, > > The get_interface_metric() function should get a more thorough rewrite > than just a compiler warning shut-up. So the patch will probably get divided > in two - the simple signed/unsigned fixes and get_interface_metric() > redesign. > > For the latter, I had "re-invented" the get_interfac

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-05 Thread Selva Nair
Hi Simon, On Tue, Dec 5, 2017 at 4:44 AM, Simon Rozman wrote: > Hi, > >> On Wed, Nov 08, 2017 at 06:46:53PM +, Simon Rozman wrote: >> > > The best time to re-factor a function would be when a a new use >> > > case needs to change its semantics. Apart from the ill-chosen -err >> > > as a retu

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-05 Thread Simon Rozman
Hi, > On Wed, Nov 08, 2017 at 06:46:53PM +, Simon Rozman wrote: > > > The best time to re-factor a function would be when a a new use > > > case needs to change its semantics. Apart from the ill-chosen -err > > > as a return value, currently it returns 0 if automatic metric is in > > > use, m

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-12-04 Thread Gert Doering
Hi, On Wed, Nov 08, 2017 at 06:46:53PM +, Simon Rozman wrote: > > The best time to re-factor a function would be when a a new use case needs > > to change its semantics. Apart from the ill-chosen -err as a return value, > > currently it returns 0 if automatic metric is in use, making it impos

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-08 Thread Simon Rozman
Hi, > The best time to re-factor a function would be when a a new use case needs > to change its semantics. Apart from the ill-chosen -err as a return value, > currently it returns 0 if automatic metric is in use, making it impossible to > use > it as a generic function to find the current metri

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-06 Thread Selva
Hi, On Mon, Nov 6, 2017 at 3:35 AM, Simon Rozman wrote: > > Hi, > > > > > > diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c > > > > > index d43cbcf..f88ba2c 100644 > > > > > --- a/src/openvpn/block_dns.c > > > > > +++ b/src/openvpn/block_dns.c > > > > > @@ -370,7 +370,7 @@ get_inte

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-06 Thread Gert Doering
Hi, On Mon, Nov 06, 2017 at 08:35:48AM +, Simon Rozman wrote: > > To repeat,  cast does not eliminate a potential for error, it just just > > hides it. > > True and I totally agree with you. I just didn't dare to rewrite the function > more thoroughly. Can I, please? Refactoring is definit

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-06 Thread Simon Rozman
Hi, > > > > diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c > > > > index d43cbcf..f88ba2c 100644 > > > > --- a/src/openvpn/block_dns.c > > > > +++ b/src/openvpn/block_dns.c > > > > @@ -370,7 +370,7 @@ get_interface_metric(const NET_IFINDEX index, > > > const ADDRESS_FAMILY family)

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-05 Thread Selva
Hi, I understand the intent here and its nice to get rid of compiler warnings, when it makes sense. But be careful while silencing by just a cast. On Sun, Nov 5, 2017 at 5:06 PM, Simon Rozman wrote: > > Hi, > > Let me explain all proposed modifications case-by-case below... > > > > diff --git a/

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-05 Thread Simon Rozman
Hi, Let me explain all proposed modifications case-by-case below... > > diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c index > > d43cbcf..f88ba2c 100644 > > --- a/src/openvpn/block_dns.c > > +++ b/src/openvpn/block_dns.c > > @@ -370,7 +370,7 @@ get_interface_metric(const NET_IFIND

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-05 Thread David Sommerseth
On 05/11/17 20:20, Gert Doering wrote: >> -modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext) >> +modext(LPTSTR dest, size_t size, LPCTSTR src, LPCTSTR newext) >> { >> int i; > Not sure why this is needed? The code verifies that size is ">0", so > a signed variable is ok here. I wou

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-05 Thread Selva
Hi, > > index 7a2d0e3..5422d33 100644 > > --- a/src/openvpnserv/validate.c > > +++ b/src/openvpnserv/validate.c > > @@ -298,7 +298,7 @@ IsUserInGroup(PSID sid, const PTOKEN_GROUPS > token_groups, const WCHAR *group_nam > > break; > > } > > /* If a match is already f

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

2017-11-05 Thread Gert Doering
Hi, On Wed, Oct 11, 2017 at 01:11:26AM +0200, si...@rozman.si wrote: > From: Simon Rozman > > --- > src/openvpn/block_dns.c | 2 +- > src/openvpnserv/automatic.c | 2 +- > src/openvpnserv/common.c | 2 +- > src/openvpnserv/interactive.c | 4 ++-- > src/openvpnserv/validate.c| 2