On Friday 18 April 2025 22:20:42 Lasse Collin wrote:
> On 2025-04-18 Pali Rohár wrote:
> > On Friday 18 April 2025 21:27:06 Lasse Collin wrote:
> > > On 2025-04-18 Pali Rohár wrote:
> > > > 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.
> > >
> > > I didn't test, but I read the code fairly carefully. It looks good,
> > > although one thing I don't understand: if i386 and x64 getenv_s is
> > > the emulated version with MSVCRT and old CRTs, isn't the problem in
> > > getopt() still there when using these CRTs (changes to environment
> > > won't be detected after process startup)?
> >
> > It should not be a problem. Implementation of emulated version of
> > getenv_s() for older CRTs uses getenv(), which we know that is
> > correct. And for msvcrt.dll builds it uses msvcrt_or_emu_glue.h which
> > dynamically use system version of getenv_s() if available in the
> > msvcrt.dll.
>
> Right. I had noticed and understood this but had forgotten it when
> writing that paragraph in the email. :-( I only had the winstorecompat
> version in my mind at that point. Sorry.
>
> > And I doubt that there are requests for Annex K in mingw-w64.
>
> Right. I was merely comparing Annex K and MS variants to be sure I
> understood things correctly.
>
> > > (3)
> > > MS docs say that errno is set for the EINVAL cases. It doesn't say
> > > it about ERANGE.
> >
> > It says, see:
> > https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/getenv-s-wgetenv-s
> >
> > "Also, if the buffer is too small, these functions return ERANGE."
>
> I meant that it doesn't say anything about errno (neither "sets errno"
> nor "does not set errno") in context of ERANGE. Your code matches the
> documentation. :-)
>
> > > I see your comments in the code. I suppose you have tested
> > > that errno really isn't set by MS CRTs after the EINVAL checks. It
> > > just feels weird.
> >
> > Yes, I checked it. But it is how it works. All these MS functions have
> > as a return value those ESOMETHING constants and these functions do
> > not set errno on error at all.
>
> Thanks. That's how it is then.
>
> > > (4)
> > > Most of winstorecompat is under the MIT license instead of being
> > > public domain. Maybe the new file in winstorecompat should be
> > > MIT-licensed too.
> > >
> > > (There already are multiple copyright/author notice variants above
> > > the MIT license text in the src/*.c files. To legally distribute
> > > binaries that link against static winstorecompat, one needs to
> > > extract all variants of those notices and make them available with
> > > the binaries...)
> >
> > I have not problem with MIT license. If there is a request for it I
> > can add it into header.
>
> It's up to the mingw-w64 maintainers. My comment was just to point out
> that the new file didn't match any other file in winstorecompat.
>
> > > (5)
> > > The commit message of patch 3 has a typo:
> > >
> > > CRT storage and and process block any be out of the sync,
> > > ^^^ may
> > >
> > > --
> > > Lasse Collin
> >
> > Ok, I will fix both duplicate "and" word and missing "may".
>
> Good catch, I missed the "and". :-)
>
> --
> Lasse Collin
I fixed issues (also another new one with WinAPI as
GetEnvironmentVariableA() returns zero also when variable value is zero
length) and sent it to the list.
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
