Re: [PATCH] msvcrt: Ignore inherited FDs with invalid handles in msvcrt_init_io().

[Piotr Caban]

Hi Paul,

The change in implementation looks good for me.

On 2/15/21 12:43 PM, Paul Gofman wrote:
> -static void test_file_inherit_child(const char* fd_s)
> +static void test_file_inherit_child(const char* fd_s, const char *handle_str)
>   {
> - HANDLE handle_value;
>  int fd = atoi(fd_s);
> - HANDLE *handle_ptr;
> - unsigned int count;
>  char buffer[32];
> - STARTUPINFOA si;
>  int ret;
> - ret =write(fd, "Success", 8);
> - GetStartupInfoA(&si);
> - count = *(unsigned *)si.lpReserved2;
> - if (handle_str && count >= 2)
> - {
> -
>    sscanf(handle_str, "%p", &handle_value);
> -
>    handle_ptr = (HANDLE *)(si.lpReserved2 + sizeof(unsigned) + count);
> -
>    ok(handle_value == handle_ptr[1], "Got unexpected handle %p.\n", handle_ptr[1]);
> - }
> -
> - count = min(count, (si.cbReserved2 - sizeof(unsigned)) / (sizeof(HANDLE) + 1));

count is never used after the last assignment. It probably makes sense 
to change the code to something like:
if (handle_str)
{
     HANDLE handle_value, handle_ptr;
     unsigned int count;
     STARTUPINFOA si;

GetStartupInfoA(&si);
     count = *(unsigned int*)si.lpReserved2;
     ok(count == 3, "count = %u\n", count);

sscanf(handle_str, "%p", &handle_value);
     ...
> - /* test inherit block with larger size */
> - handles[1] = CreateFileA( "fdopen.tst", GENERIC_READ|GENERIC_WRITE,
> -
>                          FILE_SHARE_READ | FILE_SHARE_WRITE, &sa, CREATE_ALWAYS, 0, NULL );
> - /* test inherit block with invalid handle */
> - handles[1] = INVALID_HANDLE_VALUE;
>  create_io_inherit_block( &startup, 3, handles );
> - startup.cbReserved2 += 7;
> - test_stdout_handle( &startup, cmdline, handles[1], TRUE, "large size block" );
> - CloseHandle( handles[1] );
> - DeleteFileA("fdopen.tst");

Is there any reason for removing the "test inherit block with larger 
size" test?

Thanks,
Piotr