On Saturday 25 January 2025 18:17:23 Lasse Collin wrote:
> On 2025-01-25 Pali Rohár wrote:
> > On Saturday 25 January 2025 12:39:19 Lasse Collin wrote:
> > > Other things in libmingwex seem friendly to Win9x still. With a
> > > quick search with grep, I only spotted one extremely minor thing:
> > > getopt.c calls GetEnvironmentVariableW to read POSIXLY_CORRECT.
> > > That function exists on Win98 as a stub so the program still runs.
> > > It just won't obey the POSIXLY_CORRECT environment variable on
> > > Win9x, which is not a problem at all.
> >
> > IMHO this is a bug. We should use getenv() in POSIX/CRT functions and
> > not the GetEnvironmentVariable[AW]() because the WinAPI function does
> > not address things like direct modification of 3rd main argument env,
> > or similar thing which is used in POSIX applications. There is also
> > _enviroment symbol (or function which returns pointer to this symbol)
> > which can be and also is used by windows (msvcrt) applications.
>
> I understand what you mean. Luckily POSIXLY_CORRECT is such an
> environment variable that there's no problem *in practice*. It's
> highly unlikely that the value would be changed after main() has been
> called.
>
> getopt.c used to call getenv(). It was changed in 2020 in the commit
> 8917aca09469 ("crt: use GetEnvironmentVariableW in getopt"). I suppose
> there's no perfect solution but the current way shouldn't cause any
> problems in practice. In some other place than getopt.c it could be
> different.
In attachment is my attempt to fix this problem. Could you look at it?
getenv() is replaced by the getenv_s() which is implemented via
GetEnvironmentVariableA() for UWP builds.
>From 4f4ac836f6e107be3d324307f0bdac19d07edbdc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.ro...@gmail.com>
Date: Sun, 13 Apr 2025 21:07:46 +0200
Subject: [PATCH 1/4] crt: Provide mingw-w64 emulation of getenv_s function for
all CRT import libraries
---
mingw-w64-crt/Makefile.am | 2 +
mingw-w64-crt/lib-common/msvcrt.def.in | 2 +-
mingw-w64-crt/secapi/getenv_s.c | 61 ++++++++++++++++++++++++++
3 files changed, 64 insertions(+), 1 deletion(-)
create mode 100644 mingw-w64-crt/secapi/getenv_s.c
diff --git a/mingw-w64-crt/Makefile.am b/mingw-w64-crt/Makefile.am
index 009d1056f65c..522709a27f82 100644
--- a/mingw-w64-crt/Makefile.am
+++ b/mingw-w64-crt/Makefile.am
@@ -400,6 +400,7 @@ src_msvcrt_add_x86=\
secapi/_wstrdate_s.c \
secapi/_wstrtime_s.c \
secapi/asctime_s.c \
+ secapi/getenv_s.c \
secapi/memcpy_s.c \
secapi/memmove_s.c \
secapi/rand_s.c \
@@ -934,6 +935,7 @@ src_pre_msvcr80=\
misc/wcrtomb.c \
misc/wcsnlen.c \
misc/wctob.c \
+ secapi/getenv_s.c \
stdio/_fseeki64.c \
stdio/_fstat64i32.c \
stdio/_ftelli64.c \
diff --git a/mingw-w64-crt/lib-common/msvcrt.def.in
b/mingw-w64-crt/lib-common/msvcrt.def.in
index 463917f37514..026e1ff56318 100644
--- a/mingw-w64-crt/lib-common/msvcrt.def.in
+++ b/mingw-w64-crt/lib-common/msvcrt.def.in
@@ -1803,7 +1803,7 @@ freopen_s
fscanf_s
fwprintf_s
fwscanf_s
-getenv_s
+F_ARM_ANY(getenv_s) ; i386 and x64 getenv_s replaced by emu
F_ARM_ANY(mbrlen) ; i386 and x64 mbrlen replaced by emu
F_ARM_ANY(mbrtowc) ; i386 and x64 mbrtowc replaced by emu
mbsdup_dbg
diff --git a/mingw-w64-crt/secapi/getenv_s.c b/mingw-w64-crt/secapi/getenv_s.c
new file mode 100644
index 000000000000..b2731830df98
--- /dev/null
+++ b/mingw-w64-crt/secapi/getenv_s.c
@@ -0,0 +1,61 @@
+/**
+ * This file has no copyright assigned and is placed in the Public Domain.
+ * This file is part of the mingw-w64 runtime package.
+ * No warranty is given; refer to the file DISCLAIMER.PD within this package.
+ */
+
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+
+static errno_t __cdecl emu_getenv_s(size_t *pReturnValue, char *dstBuf,
rsize_t dstSize, const char *varName)
+{
+ const char *value;
+ size_t size;
+
+ /* Only below parameter validation sets errno to EINVAL. */
+
+ if (!pReturnValue)
+ return errno = EINVAL;
+
+ if ((!dstBuf && dstSize > 0) || (dstBuf && !dstSize)) {
+ *pReturnValue = 0;
+ return errno = EINVAL;
+ }
+
+ if (!varName) {
+ *pReturnValue = 0;
+ if (dstBuf)
+ dstBuf[0] = '\0';
+ return errno = EINVAL;
+ }
+
+ /* After passing parameter validation, the errno is not changed. */
+
+ value = getenv(varName);
+ if (!value) {
+ *pReturnValue = 0;
+ if (dstBuf)
+ dstBuf[0] = '\0';
+ return 0;
+ }
+
+ size = strlen(value)+1;
+ *pReturnValue = size;
+
+ if (dstBuf) {
+ if (size > dstSize) {
+ dstBuf[0] = '\0';
+ return ERANGE;
+ }
+ memcpy(dstBuf, value, size);
+ }
+
+ return 0;
+}
+
+#define RETT errno_t
+#define FUNC getenv_s
+#define ARGS size_t *pReturnValue, char *dstBuf, rsize_t dstSize, const char
*varName
+#define CALL pReturnValue, dstBuf, dstSize, varName
+#include "msvcrt_or_emu_glue.h"
--
2.20.1
>From 4657d00489b0a7dcf45eac92524de13c0af3161b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.ro...@gmail.com>
Date: Sun, 13 Apr 2025 21:09:20 +0200
Subject: [PATCH 2/4] winstorecompat: Provide getenv_s() function
Implement getenv_s() via WinAPI GetEnvironmentVariableA() function.
WinAPI function GetEnvironmentVariableA() is available in:
Windows 8 API Sets -
https://web.archive.org/web/20131207081913/http://msdn.microsoft.com/en-us/library/windows/desktop/dn505783(v=vs.85).aspx
Windows 8.1 API Sets -
https://web.archive.org/web/20161005133126/http://msdn.microsoft.com/library/windows/desktop/dn933214.aspx
Windows 10 UWP - https://learn.microsoft.com/en-us/uwp/win32-and-com/win32-apis
---
.../winstorecompat/Makefile.am | 1 +
.../winstorecompat/src/getenv_s.c | 70 +++++++++++++++++++
2 files changed, 71 insertions(+)
create mode 100644 mingw-w64-libraries/winstorecompat/src/getenv_s.c
diff --git a/mingw-w64-libraries/winstorecompat/Makefile.am
b/mingw-w64-libraries/winstorecompat/Makefile.am
index 55fb4584111c..fc8e1ec65ece 100644
--- a/mingw-w64-libraries/winstorecompat/Makefile.am
+++ b/mingw-w64-libraries/winstorecompat/Makefile.am
@@ -25,6 +25,7 @@ libwinstorecompat_a_SOURCES = \
src/GetACP.c \
src/VirtualProtect.c \
src/getenv.c \
+ src/getenv_s.c \
src/LocalAlloc.c \
src/LocalFree.c \
src/Sleep.c \
diff --git a/mingw-w64-libraries/winstorecompat/src/getenv_s.c
b/mingw-w64-libraries/winstorecompat/src/getenv_s.c
new file mode 100644
index 000000000000..68b378d398eb
--- /dev/null
+++ b/mingw-w64-libraries/winstorecompat/src/getenv_s.c
@@ -0,0 +1,70 @@
+/**
+ * This file has no copyright assigned and is placed in the Public Domain.
+ * This file is part of the mingw-w64 runtime package.
+ * No warranty is given; refer to the file DISCLAIMER.PD within this package.
+ */
+
+#include <stdlib.h>
+#include <errno.h>
+#include <windows.h>
+
+errno_t __cdecl getenv_s(size_t *pReturnValue, char *dstBuf, rsize_t dstSize,
const char *varName)
+{
+ DWORD ret;
+
+ /* Only below parameter validation sets errno to EINVAL. */
+
+ if (!pReturnValue)
+ return errno = EINVAL;
+
+ if ((!dstBuf && dstSize > 0) || (dstBuf && !dstSize)) {
+ *pReturnValue = 0;
+ return errno = EINVAL;
+ }
+
+ if (!varName) {
+ *pReturnValue = 0;
+ if (dstBuf)
+ dstBuf[0] = '\0';
+ return errno = EINVAL;
+ }
+
+ /* After passing parameter validation, the errno is not changed. */
+
+ /*
+ * Function GetEnvironmentVariableA() is documented on:
+ *
https://learn.microsoft.com/en-us/windows/win32/api/processenv/nf-processenv-getenvironmentvariablea
+ */
+ ret = GetEnvironmentVariableA(varName, dstBuf, dstSize);
+ if (ret == 0) {
+ /* If the function fails, the return value is zero (e.g. specified
+ * environment variable was not found).
+ */
+ *pReturnValue = 0;
+ if (dstBuf)
+ dstBuf[0] = '\0';
+ ret = GetLastError();
+ if (ret == ERROR_ENVVAR_NOT_FOUND)
+ return 0;
+ else if (ret == ERROR_NOT_ENOUGH_MEMORY)
+ return ENOMEM;
+ else
+ return EINVAL;
+ } else if (ret > dstSize) {
+ /* If buffer is not large enough to hold the data, the return value is
+ * the buffer size, in characters, required to hold the string and its
+ * terminating null character and the contents of buffer are undefined.
+ */
+ *pReturnValue = ret;
+ if (dstBuf)
+ dstBuf[0] = '\0';
+ return ERANGE;
+ } else {
+ /* If the GetEnvironmentVariable succeeds, the return value is the
+ * number of characters stored in the buffer, not including the
+ * terminating null character.
+ */
+ *pReturnValue = ret+1;
+ return 0;
+ }
+}
--
2.20.1
>From e1b77627bf4c5f5d0c979e17d507253e3b9919c7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.ro...@gmail.com>
Date: Sun, 13 Apr 2025 21:04:10 +0200
Subject: [PATCH 3/4] Revert "crt: use GetEnvironmentVariableW in getopt"
This reverts commit 8917aca09469d45a4c0c6fe58b36a8eb5b572836.
WinAPI function GetEnvironmentVariableW() should not be used in CRT code as
it reads the variable from the process block, not from the CRT storage.
CRT storage and and process block any be out of the sync, for example when
using the CRT's _environ[] access or the 3rd argument envp[] of the main()
function.
CRT getenv() function for CRT builds always returns correct value, so use
it instead of the WinAPI GetEnvironmentVariableW().
---
mingw-w64-crt/misc/getopt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mingw-w64-crt/misc/getopt.c b/mingw-w64-crt/misc/getopt.c
index fc317e34e84f..01b269d0a4d9 100644
--- a/mingw-w64-crt/misc/getopt.c
+++ b/mingw-w64-crt/misc/getopt.c
@@ -339,7 +339,7 @@ getopt_internal(int nargc, char * const *nargv, const char
*options,
* optreset != 0 for GNU compatibility.
*/
if (posixly_correct == -1 || optreset != 0)
- posixly_correct = (GetEnvironmentVariableW(L"POSIXLY_CORRECT",
NULL, 0) != 0);
+ posixly_correct = (getenv("POSIXLY_CORRECT") != NULL);
if (*options == '-')
flags |= FLAG_ALLARGS;
else if (posixly_correct || *options == '+')
--
2.20.1
>From 1b1f77f11664e12cc5a01aa3595c008b14b46574 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pali=20Roh=C3=A1r?= <pali.ro...@gmail.com>
Date: Sun, 13 Apr 2025 21:13:17 +0200
Subject: [PATCH 4/4] crt: getopt: Use getenv_s() instead of getenv()
mingw-w64's winstorecompat provides just dummy getenv() implementation
which always returns NULL. So it is not very usable for winstorecompat
UWP builds.
But mingw-w64's winstorecompat provides getenv_s() which returns the real
environment variable content.
So replace getenv() call by the getenv_s() which makes getopt() to work in
both normal and UWP builds.
---
mingw-w64-crt/misc/getopt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mingw-w64-crt/misc/getopt.c b/mingw-w64-crt/misc/getopt.c
index 01b269d0a4d9..5fb7868429ad 100644
--- a/mingw-w64-crt/misc/getopt.c
+++ b/mingw-w64-crt/misc/getopt.c
@@ -319,6 +319,7 @@ getopt_internal(int nargc, char * const *nargv, const char
*options,
{
char *oli; /* option letter list index */
int optchar, short_too;
+ size_t var_size;
static int posixly_correct = -1;
if (options == NULL)
@@ -339,7 +340,7 @@ getopt_internal(int nargc, char * const *nargv, const char
*options,
* optreset != 0 for GNU compatibility.
*/
if (posixly_correct == -1 || optreset != 0)
- posixly_correct = (getenv("POSIXLY_CORRECT") != NULL);
+ posixly_correct = (getenv_s(&var_size, NULL, 0,
"POSIXLY_CORRECT") == 0 && var_size > 0);
if (*options == '-')
flags |= FLAG_ALLARGS;
else if (posixly_correct || *options == '+')
--
2.20.1
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
