Hi, On Wed, Oct 11, 2017 at 01:11:26AM +0200, si...@rozman.si wrote: > From: Simon Rozman <si...@rozman.si> > > --- > 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 +- > 5 files changed, 6 insertions(+), 6 deletions(-)
So... trying to make sense of this.
> 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)
> }
> return ipiface.Metric;
> }
> - return -err;
> + return -(int)err;
> }
This, I can somewhat agree to, as "err" is an unsigned type so there
might be a hidden integer overflow if it happens to be "large". Which it
won't be, but still.
>
> /*
> diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
> index 4123d0f..6c39aaa 100644
> --- a/src/openvpnserv/automatic.c
> +++ b/src/openvpnserv/automatic.c
> @@ -155,7 +155,7 @@ match(const WIN32_FIND_DATA *find, LPCTSTR ext)
> * Modify the extension on a filename.
> */
> static bool
> -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.
>
> diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
> index b8b817b..7901fd6 100644
> --- a/src/openvpnserv/common.c
> +++ b/src/openvpnserv/common.c
> @@ -36,7 +36,7 @@ openvpn_vsntprintf(LPTSTR str, size_t size, LPCTSTR format,
> va_list arglist)
> len = _vsntprintf(str, size, format, arglist);
> str[size - 1] = 0;
> }
> - return (len >= 0 && len < size);
> + return (len >= 0 && (size_t)len < size);
> }
This is, if I understand right, because "len < size" will implicitly cast
size to (int), causing integer overflow if size is too big for a signed
int?
> int
> openvpn_sntprintf(LPTSTR str, size_t size, LPCTSTR format, ...)
> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> index 8d94197..96e0de0 100644
> --- a/src/openvpnserv/interactive.c
> +++ b/src/openvpnserv/interactive.c
> @@ -188,7 +188,7 @@ typedef enum {
> static DWORD
> AsyncPipeOp(async_op_t op, HANDLE pipe, LPVOID buffer, DWORD size, DWORD
> count, LPHANDLE events)
> {
> - int i;
> + DWORD i;
... and this totally escapes me. Why would an "int" suddenly be no longer
a legal loop variable...?
> BOOL success;
> HANDLE io_event;
> DWORD res, bytes = 0;
> @@ -1061,7 +1061,7 @@ RegisterDNS(LPVOID unused)
> { ipcfg, L"ipconfig /flushdns", timeout },
> { ipcfg, L"ipconfig /registerdns", timeout },
> };
> - int ncmds = sizeof(cmds) / sizeof(cmds[0]);
> + DWORD ncmds = sizeof(cmds) / sizeof(cmds[0]);
Same thing here...
> 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 found, ret == TRUE and the loop is skipped
> */
> - for (int i = 0; i < nread && !ret; ++i)
> + for (DWORD i = 0; i < nread && !ret; ++i)
... and here.
Consider me not convinced...
what exactly are the warnings MSVC spits out here?
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
signature.asc
Description: PGP signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
