In my opinion we're far too quiet about mmap and allocation failures that will likely be fatal to the application. It generally takes some close inspection of +virtual or even +relay to detect running out of address space, e.g..
In my testing the added logs here shouldn't be too noisy - I've avoided them in the `try` functions and in cases where an error code is normal. I made the cross-process allocation failure a warning, because the target process itself will presumably log an error if it goes wrong.
I also explicitly logged errno in a few cases where we assume its value. That can help catch some exotic error cases.
--
v2: ntdll: Add error and warning logs in more cases of memory exhaustion.
https://gitlab.winehq.org/wine/wine/-/merge_requests/3598
Even though the uninitialized values shouldn't be used in the output
binary program, they still show up in it, and affect the checksum, so
we better make them zeroes.
This error was reported by valgrind:
```
libtool --mode=execute valgrind --track-origins=yes vkd3d-build/tests/shader_runner vkd3d/tests/hlsl/static-initializer.shader_test
```
```
==46180== Conditional jump or move depends on uninitialised value(s)
==46180== at 0x48D98C7: parse_dxbc.isra.0 (dxbc.c:182)
==46180== by 0x48DA044: vkd3d_shader_parse_dxbc (dxbc.c:308)
==46180== by 0x488D1B6: vkd3d_shader_parse_dxbc_source_type (vkd3d_shader_utils.h:37)
==46180== by 0x488D1B6: create_shader_stage.isra.0 (state.c:1988)
==46180== by 0x48926B6: d3d12_pipeline_state_init_graphics (state.c:3084)
==46180== by 0x4893A96: d3d12_pipeline_state_create_graphics (state.c:3280)
==46180== by 0x4878498: d3d12_device_CreateGraphicsPipelineState (device.c:2619)
==46180== by 0x1FFEFFECC7: ???
==46180== by 0xE8: ???
==46180== by 0x47: ???
==46180== by 0x61: ???
==46180== by 0x660066000000023: ???
==46180== by 0x661066100000044: ???
==46180== Uninitialised value was created by a stack allocation
==46180== at 0x48F3FF0: hlsl_fold_constant_swizzles (hlsl_constant_ops.c:1010)
```
Thank you valgrind! :smile:
--
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/308
In my opinion we're far too quiet about mmap and allocation failures that will likely be fatal to the application. It generally takes some close inspection of +virtual or even +relay to detect running out of address space, e.g..
In my testing the added logs here shouldn't be too noisy - I've avoided them in the `try` functions and in cases where an error code is normal. I made the cross-process allocation failure a warning, because the target process itself will presumably log an error if it goes wrong.
I also explicitly logged errno in a few cases where we assume its value. That can help catch some exotic error cases.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3598
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
> }
> }
>
> + if (session->state == SESSION_STATE_STARTED)
> + {
> + session_flush_transform_output_nodes(session);
> + session_set_source_output_nodes_seeking(session);
> + }
Same here, isn't it possible to seek from paused state too?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3572#note_42633
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
>
> /* fallthrough */
> case SESSION_STATE_PAUSED:
> + case SESSION_STATE_STARTED:
> + if (session->state == SESSION_STATE_STARTED && !(session->caps & MFSESSIONCAP_SEEK))
> + {
> + WARN("Seeking is not supported for this session.\n");
> + session_command_complete(session);
> + return;
> + }
Why does it check for STARTED state?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3572#note_42632
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
> return hr;
> }
>
> +static void session_flush_transform_output_nodes(struct media_session *session)
This could be call session_flush_nodes(), or flush_topology/flush_pipeline, no need to make the name that specific.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3572#note_42631