diff mbox series

[Openvpn-devel,3/5] interactive.c: Resolve MSVC C4996 warning

Message ID 20210321144627.1621-3-simon@rozman.si
State Accepted
Headers show
Series [Openvpn-devel,1/5] MSVC: Disable LZ4 | expand

Commit Message

Kristof Provost via Openvpn-devel March 21, 2021, 3:46 a.m. UTC 
It's about using a standard recommended alias for the wcsdup():

> warning C4996: 'wcsdup': The POSIX name for this item is deprecated.
> Instead, use the ISO C and C++ conformant name: _wcsdup. See online
> help for details.

And the documentation says:

> The Microsoft-implemented POSIX function names strdup and wcsdup are
> deprecated aliases for the _strdup and _wcsdup functions. By default,
> they generate Compiler warning (level 3) C4996. The names are
> deprecated because they don't follow the Standard C rules for
> implementation-specific names. However, the functions are still
> supported.
>
> We recommend you use _strdup and _wcsdup instead. Or, you can continue
> to use these function names, and disable the warning. For more
> information, see Turn off the warning and POSIX function names.

Reference: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strdup-wcsdup
Signed-off-by: Simon Rozman <simon@rozman.si>
---
 src/openvpnserv/interactive.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Arne Schwabe March 21, 2021, 5:38 a.m. UTC | #1 
Am 21.03.21 um 15:46 schrieb Simon Rozman via Openvpn-devel:
> It's about using a standard recommended alias for the wcsdup():
> 
>> warning C4996: 'wcsdup': The POSIX name for this item is deprecated.
>> Instead, use the ISO C and C++ conformant name: _wcsdup. See online
>> help for details.
> 
> And the documentation says:
> 
>> The Microsoft-implemented POSIX function names strdup and wcsdup are
>> deprecated aliases for the _strdup and _wcsdup functions. By default,
>> they generate Compiler warning (level 3) C4996. The names are
>> deprecated because they don't follow the Standard C rules for
>> implementation-specific names. However, the functions are still
>> supported.
>>
>> We recommend you use _strdup and _wcsdup instead. Or, you can continue
>> to use these function names, and disable the warning. For more
>> information, see Turn off the warning and POSIX function names.
> 
> Reference: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strdup-wcsdup
> Signed-off-by: Simon Rozman <simon@rozman.si>
> ---
>  src/openvpnserv/interactive.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> index 5d5cbfe6..b073a0d5 100644
> --- a/src/openvpnserv/interactive.c
> +++ b/src/openvpnserv/interactive.c
> @@ -899,7 +899,7 @@ ExecCommand(const WCHAR *argv0, const WCHAR *cmdline, DWORD timeout)
>      si.cb = sizeof(si);
>  
>      /* CreateProcess needs a modifiable cmdline: make a copy */
> -    cmdline_dup = wcsdup(cmdline);
> +    cmdline_dup = _wcsdup(cmdline);
>      if (cmdline_dup && CreateProcessW(argv0, cmdline_dup, NULL, NULL, FALSE,
>                                        proc_flags, NULL, NULL, &si, &pi) )
>      {
> @@ -1181,7 +1181,7 @@ SetDNSDomain(const wchar_t *if_name, const char *domain, undo_lists_t *lists)
>     /* Add to undo list if domain is non-empty */
>     if (err == 0 && wdomain[0] && lists)
>     {
> -        wchar_t *tmp_name = wcsdup(if_name);
> +        wchar_t *tmp_name = _wcsdup(if_name);
>          if (!tmp_name || AddListItem(&(*lists)[undo_domain], tmp_name))
>          {
>              free(tmp_name);
> @@ -1272,7 +1272,7 @@ HandleDNSConfigMessage(const dns_cfg_message_t *msg, undo_lists_t *lists)
>  
>      if (msg->addr_len > 0)
>      {
> -        wchar_t *tmp_name = wcsdup(wide_name);
> +        wchar_t *tmp_name = _wcsdup(wide_name);
>          if (!tmp_name || AddListItem(&(*lists)[undo_type], tmp_name))
>          {
>              free(tmp_name);
> 

Acked-By: Arne Schwabe <arne@rfc2549.org>

I can accept the change in this function since it is windows specific
but overall I don't think we can reasonable move away from POSIX
function names and instead use the names with _ in front of it because
that is currently Windows specific. I am not sure what the idea for a
portable code is other than to set the _CRT_NONSTDC_NO_DEPRECATE option
on Windows.

Arne
Gert Doering March 21, 2021, 6:05 a.m. UTC | #2 
I seem to remember we had that discussion in the context of openpvpn-gui
already - Problems with the "POSIX compatible" function names, which end
up acting on narrow or wide strings depending on compiler settings, compiler
version, phase of the moon...  can't find that commit, but if MS docs says
"this is how windows code should look like", so be it...

I have test compiled stuff on Ubuntu 18 / MinGW, and it compiles.  Not tested.

Your patch has been applied to the master branch.

commit 709c3810a1d67e2c4049e852529a0a0d1338c797
Author: Simon Rozman via Openvpn-devel
Date:   Sun Mar 21 15:46:25 2021 +0100

     interactive.c: Resolve MSVC C4996 warning

     Signed-off-by: Simon Rozman <simon@rozman.si>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20210321144627.1621-3-simon@rozman.si>
     URL: https://www.mail-archive.com/search?l=mid&q=20210321144627.1621-3-simon@rozman.si
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Gert Doering March 21, 2021, 6:11 a.m. UTC | #3 
Hi,

On Sun, Mar 21, 2021 at 06:05:19PM +0100, Gert Doering wrote:
> commit 709c3810a1d67e2c4049e852529a0a0d1338c797
> Author: Simon Rozman via Openvpn-devel

Yeah.  Right.

So, apologies for not catching this in time and fixing the Author: line,
but this is exactly why I brought up the DMARC issue with Max Fillinger
yesterday.

And it royally messes up patchwork as well, because patchwork only cares
about mail addresses internally, and shows the name "last seen with that
address" - so the Patch from Steffan / Max is now listed as

"Simon Rozman via Openvpn-devel"

as well... https://patchwork.openvpn.net/patch/1628/

*sigh*

gert
diff mbox series

Patch

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 5d5cbfe6..b073a0d5 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -899,7 +899,7 @@  ExecCommand(const WCHAR *argv0, const WCHAR *cmdline, DWORD timeout)
     si.cb = sizeof(si);
 
     /* CreateProcess needs a modifiable cmdline: make a copy */
-    cmdline_dup = wcsdup(cmdline);
+    cmdline_dup = _wcsdup(cmdline);
     if (cmdline_dup && CreateProcessW(argv0, cmdline_dup, NULL, NULL, FALSE,
                                       proc_flags, NULL, NULL, &si, &pi) )
     {
@@ -1181,7 +1181,7 @@  SetDNSDomain(const wchar_t *if_name, const char *domain, undo_lists_t *lists)
    /* Add to undo list if domain is non-empty */
    if (err == 0 && wdomain[0] && lists)
    {
-        wchar_t *tmp_name = wcsdup(if_name);
+        wchar_t *tmp_name = _wcsdup(if_name);
         if (!tmp_name || AddListItem(&(*lists)[undo_domain], tmp_name))
         {
             free(tmp_name);
@@ -1272,7 +1272,7 @@  HandleDNSConfigMessage(const dns_cfg_message_t *msg, undo_lists_t *lists)
 
     if (msg->addr_len > 0)
     {
-        wchar_t *tmp_name = wcsdup(wide_name);
+        wchar_t *tmp_name = _wcsdup(wide_name);
         if (!tmp_name || AddListItem(&(*lists)[undo_type], tmp_name))
         {
             free(tmp_name);