| Message ID | 20171113094952.33364-1-simon@rozman.si |
|---|---|
| State | Superseded |
| Delegated to: | Selva Nair |
| Headers | show |
| Series | [Openvpn-devel,v2] openvpnserv: Review MSVC down-casting warnings | expand |
Hi, Thanks for the v2 On Mon, Nov 13, 2017 at 4:49 AM, Simon Rozman <simon@rozman.si> wrote: > Data size arithmetic was reviewed according to 64-bit MSVC complaints. > > The warnings were addressed by migrating to size_t, rewriting the code, > or silencing the warnings by an explicit cast where appropriate. > --- > src/openvpnserv/automatic.c | 20 ++++---------------- > src/openvpnserv/interactive.c | 12 ++++++------ > 2 files changed, 10 insertions(+), 22 deletions(-) > > diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c > index 75c3be2..4d5501b 100644 > --- a/src/openvpnserv/automatic.c > +++ b/src/openvpnserv/automatic.c > @@ -115,25 +115,13 @@ close_if_open(HANDLE h) > static bool > match(const WIN32_FIND_DATA *find, LPCTSTR ext) > { > - int i; > - > if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) > { > return false; > } > > - if (!_tcslen(ext)) > - { > - return true; > - } > - > - i = _tcslen(find->cFileName) - _tcslen(ext) - 1; > - if (i < 1) > - { > - return false; > - } > - > - return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + > 1, ext); > + const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.')); > + return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0; FWIW, ensuring the extension string is not empty seems to be required to avoid a regression (see my reply to v1). A check for ext[0] != TEXT('\0') should do the trick. The rest looks good. Selva <div dir="ltr">Hi,<div><br></div><div>Thanks for the v2<br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 13, 2017 at 4:49 AM, Simon Rozman <span dir="ltr"><<a href="mailto:simon@rozman.si" target="_blank">simon@rozman.si</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Data size arithmetic was reviewed according to 64-bit MSVC complaints.<br> <br> The warnings were addressed by migrating to size_t, rewriting the code,<br> or silencing the warnings by an explicit cast where appropriate.<br> ---<br> src/openvpnserv/automatic.c | 20 ++++----------------<br> src/openvpnserv/interactive.c | 12 ++++++------<br> 2 files changed, 10 insertions(+), 22 deletions(-)<br> <br> diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c<br> index 75c3be2..4d5501b 100644<br> --- a/src/openvpnserv/automatic.c<br> +++ b/src/openvpnserv/automatic.c<br> @@ -115,25 +115,13 @@ close_if_open(HANDLE h)<br> static bool<br> match(const WIN32_FIND_DATA *find, LPCTSTR ext)<br> {<br> - int i;<br> -<br> if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)<br> {<br> return false;<br> }<br> <br> - if (!_tcslen(ext))<br> - {<br> - return true;<br> - }<br> -<br> - i = _tcslen(find->cFileName) - _tcslen(ext) - 1;<br> - if (i < 1)<br> - {<br> - return false;<br> - }<br> -<br> - return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + 1, ext);<br> + const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.'));<br> + return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0;</blockquote><div><br></div><div>FWIW, ensuring the extension string is not empty seems to be required to</div><div>avoid a regression (see my reply to v1). A check for ext[0] != TEXT('\0')</div><div>should do the trick.</div><div><br></div><div>The rest looks good.</div><div><br></div><div>Selva</div></div></div></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi Simon, And this one: On Mon, Nov 13, 2017 at 11:26 AM, Selva <selva.nair@gmail.com> wrote: > Hi, > > Thanks for the v2 > > On Mon, Nov 13, 2017 at 4:49 AM, Simon Rozman <simon@rozman.si> wrote: > >> Data size arithmetic was reviewed according to 64-bit MSVC complaints. >> >> The warnings were addressed by migrating to size_t, rewriting the code, >> or silencing the warnings by an explicit cast where appropriate. >> --- >> src/openvpnserv/automatic.c | 20 ++++---------------- >> src/openvpnserv/interactive.c | 12 ++++++------ >> 2 files changed, 10 insertions(+), 22 deletions(-) >> >> diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c >> index 75c3be2..4d5501b 100644 >> --- a/src/openvpnserv/automatic.c >> +++ b/src/openvpnserv/automatic.c >> @@ -115,25 +115,13 @@ close_if_open(HANDLE h) >> static bool >> match(const WIN32_FIND_DATA *find, LPCTSTR ext) >> { >> - int i; >> - >> if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) >> { >> return false; >> } >> >> - if (!_tcslen(ext)) >> - { >> - return true; >> - } >> - >> - i = _tcslen(find->cFileName) - _tcslen(ext) - 1; >> - if (i < 1) >> - { >> - return false; >> - } >> - >> - return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + >> 1, ext); >> + const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.')); >> + return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0; > > > FWIW, ensuring the extension string is not empty seems to be required to > avoid a regression (see my reply to v1). A check for ext[0] != TEXT('\0') > should do the trick. > > The rest looks good. > Needs a v3, I suppose. Thanks, Selva <div dir="ltr">Hi Simon,<div><br></div><div>And this one:</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 13, 2017 at 11:26 AM, Selva <span dir="ltr"><<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div>Thanks for the v2<br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Mon, Nov 13, 2017 at 4:49 AM, Simon Rozman <span dir="ltr"><<a href="mailto:simon@rozman.si" target="_blank">simon@rozman.si</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Data size arithmetic was reviewed according to 64-bit MSVC complaints.<br> <br> The warnings were addressed by migrating to size_t, rewriting the code,<br> or silencing the warnings by an explicit cast where appropriate.<br> ---<br> src/openvpnserv/automatic.c | 20 ++++----------------<br> src/openvpnserv/interactive.c | 12 ++++++------<br> 2 files changed, 10 insertions(+), 22 deletions(-)<br> <br> diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c<br> index 75c3be2..4d5501b 100644<br> --- a/src/openvpnserv/automatic.c<br> +++ b/src/openvpnserv/automatic.c<br> @@ -115,25 +115,13 @@ close_if_open(HANDLE h)<br> static bool<br> match(const WIN32_FIND_DATA *find, LPCTSTR ext)<br> {<br> - int i;<br> -<br> if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)<br> {<br> return false;<br> }<br> <br> - if (!_tcslen(ext))<br> - {<br> - return true;<br> - }<br> -<br> - i = _tcslen(find->cFileName) - _tcslen(ext) - 1;<br> - if (i < 1)<br> - {<br> - return false;<br> - }<br> -<br> - return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + 1, ext);<br> + const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.'));<br> + return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0;</blockquote><div><br></div></div></div><div>FWIW, ensuring the extension string is not empty seems to be required to</div><div>avoid a regression (see my reply to v1). A check for ext[0] != TEXT('\0')</div><div>should do the trick.</div><div><br></div><div>The rest looks good.</div></div></div></div></div></blockquote><div><br></div><div>Needs a v3, I suppose.</div><div><br></div><div>Thanks,</div><div><br></div><div>Selva </div></div><br></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c index 75c3be2..4d5501b 100644 --- a/src/openvpnserv/automatic.c +++ b/src/openvpnserv/automatic.c @@ -115,25 +115,13 @@ close_if_open(HANDLE h) static bool match(const WIN32_FIND_DATA *find, LPCTSTR ext) { - int i; - if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { return false; } - if (!_tcslen(ext)) - { - return true; - } - - i = _tcslen(find->cFileName) - _tcslen(ext) - 1; - if (i < 1) - { - return false; - } - - return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + 1, ext); + const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.')); + return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0; } /* @@ -142,14 +130,14 @@ match(const WIN32_FIND_DATA *find, LPCTSTR ext) static bool modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext) { - int i; + size_t i; if (size > 0 && (_tcslen(src) + 1) <= size) { _tcscpy(dest, src); dest [size - 1] = TEXT('\0'); i = _tcslen(dest); - while (--i >= 0) + while (i-- > 0) { if (dest[i] == TEXT('\\')) { diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index ed4603e..5d58ceb 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -280,7 +280,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD count, LPHANDLE events) swprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg); buf[_countof(buf) - 1] = '\0'; - WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events); + WritePipeAsync(pipe, buf, (DWORD)(wcslen(buf) * 2), count, events); } static VOID @@ -308,7 +308,7 @@ ReturnError(HANDLE pipe, DWORD error, LPCWSTR func, DWORD count, LPHANDLE events L"0x%1!08x!\n%2!s!\n%3!s!", 0, 0, (LPWSTR) &result, 0, (va_list *) args); - WritePipeAsync(pipe, result, wcslen(result) * 2, count, events); + WritePipeAsync(pipe, result, (DWORD)(wcslen(result) * 2), count, events); #ifdef UNICODE MsgToEventLog(MSG_FLAGS_ERROR, result); #else @@ -452,10 +452,10 @@ out: static BOOL GetStartupData(HANDLE pipe, STARTUP_DATA *sud) { - size_t len; + size_t size, len; BOOL ret = FALSE; WCHAR *data = NULL; - DWORD size, bytes, read; + DWORD bytes, read; bytes = PeekNamedPipeAsync(pipe, 1, &exit_event); if (bytes == 0) @@ -1051,7 +1051,7 @@ netsh_dns_cmd(const wchar_t *action, const wchar_t *proto, const wchar_t *if_nam const wchar_t *fmt = L"netsh interface %s %s dns \"%s\" %s"; /* max cmdline length in wchars -- include room for worst case and some */ - int ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1; + size_t ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1; wchar_t *cmdline = malloc(ncmdline*sizeof(wchar_t)); if (!cmdline) { @@ -1571,7 +1571,7 @@ RunOpenvpn(LPVOID p) { DWORD written; WideCharToMultiByte(CP_UTF8, 0, sud.std_input, -1, input, input_size, NULL, NULL); - WriteFile(stdin_write, input, strlen(input), &written, NULL); + WriteFile(stdin_write, input, (DWORD)strlen(input), &written, NULL); free(input); }