Re: MPS: Win64 testers?

[Quang Kien Nguyen <kien.n.quang@gmail.com>]

> Yes, there is. It makes assumptions about the API implementation that
> valid implementations of the Windows API do not satisfy. Wine until last
> week is one example; given that the bug wasn't reported in 20 years, we
> can assume that no other popular application relies on this particular
> detail, so relying on it even in MSVCRT builds is unsafe.
> IMHO, the only thing the first patch is missing is a comment reminding
> us to remove the old code if and when support for Windows 95/98 is
> dropped.

Since the UCRT is the replacement of MSVCRT, just putting that behind
an ifdef without
checking for the function pointer is also fine to me.  Eventually, the
UCRT, which
is receiving all of the security fixes, should be replacing the MSVCRT.

On Wed, Aug 7, 2024 at 12:50 PM Pip Cet <pipcet@protonmail.com> wrote:
>
> "Eli Zaretskii" <eliz@gnu.org> writes:
>
> >> From: Quang Kien Nguyen <kien.n.quang@gmail.com>
> >> Date: Tue, 6 Aug 2024 13:52:27 -0700
> >> Cc: emacs-devel@gnu.org, pipcet@protonmail.com
> >> @@ -10480,60 +10480,73 @@ init_ntproc (int dumping)
> >>    /* Initial preparation for subprocess support: replace our standard
> >>       handles with non-inheritable versions. */
> >>    {
> >> -    HANDLE parent;
> >> -    HANDLE stdin_save =  INVALID_HANDLE_VALUE;
> >> -    HANDLE stdout_save = INVALID_HANDLE_VALUE;
> >> -    HANDLE stderr_save = INVALID_HANDLE_VALUE;
> >> -
> >> -    parent = GetCurrentProcess ();
> >> -
> >> -    /* ignore errors when duplicating and closing; typically the
> >> -       handles will be invalid when running as a gui program. */
> >> -    DuplicateHandle (parent,
> >> -                 GetStdHandle (STD_INPUT_HANDLE),
> >> -                 parent,
> >> -                 &stdin_save,
> >> -                 0,
> >> -                 FALSE,
> >> -                 DUPLICATE_SAME_ACCESS);
> >> -
> >> -    DuplicateHandle (parent,
> >> -                 GetStdHandle (STD_OUTPUT_HANDLE),
> >> -                 parent,
> >> -                 &stdout_save,
> >> -                 0,
> >> -                 FALSE,
> >> -                 DUPLICATE_SAME_ACCESS);
> >> -
> >> -    DuplicateHandle (parent,
> >> -                 GetStdHandle (STD_ERROR_HANDLE),
> >> -                 parent,
> >> -                 &stderr_save,
> >> -                 0,
> >> -                 FALSE,
> >> -                 DUPLICATE_SAME_ACCESS);
> >> -
> >> -    fclose (stdin);
> >> -    fclose (stdout);
> >> -    fclose (stderr);
> >> -
> >> -    if (stdin_save != INVALID_HANDLE_VALUE)
> >> -      _open_osfhandle ((intptr_t) stdin_save, O_TEXT);
> >> +    /* For UCRT, the _fdopen will try to find free stream from
> >> +       _IOB_ENTRIES (= 3), thus we can't reopen the standard handles
> >> +       with it. Using SetHandleInformation to make the handle not
> >> +       inheritable to child process is a better way. */
> >
> > This is not what I think we need.  First, there's no need to change
> > anything in the MSVCRT build, because AFAIK the current code "just
>
> Yes, there is. It makes assumptions about the API implementation that
> valid implementations of the Windows API do not satisfy. Wine until last
> week is one example; given that the bug wasn't reported in 20 years, we
> can assume that no other popular application relies on this particular
> detail, so relying on it even in MSVCRT builds is unsafe.
>
> IMHO, the only thing the first patch is missing is a comment reminding
> us to remove the old code if and when support for Windows 95/98 is
> dropped.
>
> Pip
>


-- 
Best regards,
Kien