[PATCH v2 0/3] MR6454: msvcrt: Some FIXME once stuff.

[Aida Jonikienė (＠DodoGTA)]

CLIP STUDIO PAINT spams FIXMEs for these functions when starting up.

--
  v2: msvcrt: Only print FIXME once for Context_Yield().
      msvcrt: Remove FIXME for _StructuredTaskCollection_dtor().
      msvcrt: Print less FIXMEs for ThreadScheduler_ScheduleTask*().

https://gitlab.winehq.org/wine/wine/-/merge_requests/6454

[PATCH v2 1/3] msvcrt: Print less FIXMEs for ThreadScheduler_ScheduleTask*().

[Aida Jonikienė]

From: Aida Jonikienė

---
 dlls/msvcrt/concurrency.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c
index 39059d66b38..8f99fd0c4de 100644
--- a/dlls/msvcrt/concurrency.c
+++ b/dlls/msvcrt/concurrency.c
@@ -1557,10 +1557,14 @@ DEFINE_THISCALL_WRAPPER(ThreadScheduler_ScheduleTask_loc, 16)
 void __thiscall ThreadScheduler_ScheduleTask_loc(ThreadScheduler *this,
         void (__cdecl *proc)(void*), void* data, /*location*/void *placement)
 {
+    static unsigned int once;
     schedule_task_arg *arg;
     TP_WORK *work;

-    FIXME("(%p %p %p %p) stub\n", this, proc, data, placement);
+    if(!once++)
+        FIXME("(%p %p %p %p) semi-stub\n", this, proc, data, placement);
+    else
+        TRACE("(%p %p %p %p) semi-stub\n", this, proc, data, placement);

arg = operator_new(sizeof(*arg));
     arg->proc = proc;
@@ -1586,7 +1590,7 @@ DEFINE_THISCALL_WRAPPER(ThreadScheduler_ScheduleTask, 12)
 void __thiscall ThreadScheduler_ScheduleTask(ThreadScheduler *this,
         void (__cdecl *proc)(void*), void* data)
 {
-    FIXME("(%p %p %p) stub\n", this, proc, data);
+    TRACE("(%p %p %p)\n", this, proc, data);
     ThreadScheduler_ScheduleTask_loc(this, proc, data, NULL);
 }

[PATCH v2 2/3] msvcrt: Remove FIXME for _StructuredTaskCollection_dtor().

[Aida Jonikienė]

From: Aida Jonikienė

---
 dlls/msvcrt/concurrency.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c
index 8f99fd0c4de..220ea35fffa 100644
--- a/dlls/msvcrt/concurrency.c
+++ b/dlls/msvcrt/concurrency.c
@@ -2096,7 +2096,8 @@ _StructuredTaskCollection* __thiscall _StructuredTaskCollection_ctor(
 DEFINE_THISCALL_WRAPPER(_StructuredTaskCollection_dtor, 4)
 void __thiscall _StructuredTaskCollection_dtor(_StructuredTaskCollection *this)
 {
-    FIXME("(%p): stub!\n", this);
+    TRACE("(%p)\n", this);
+
     if (this->count && !__uncaught_exception()) {
         missing_wait e;
         missing_wait_ctor_str(&e, "Missing call to _RunAndWait");

[PATCH v2 3/3] msvcrt: Only print FIXME once for Context_Yield().

[Aida Jonikienė]

From: Aida Jonikienė

---
 dlls/msvcrt/concurrency.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/dlls/msvcrt/concurrency.c b/dlls/msvcrt/concurrency.c
index 220ea35fffa..561b4d23289 100644
--- a/dlls/msvcrt/concurrency.c
+++ b/dlls/msvcrt/concurrency.c
@@ -861,7 +861,10 @@ void __cdecl Context_Block(void)
 /* ?_Yield@_Context@details@Concurrency@@SAXXZ */
 void __cdecl Context_Yield(void)
 {
-    FIXME("()\n");
+    static unsigned int once;
+
+    if (!once++)
+        FIXME("()\n");
 }

/* ?_SpinYield@Context@Concurrency@@SAXXZ */

[Aida Jonikienė (＠DodoGTA)]

On Fri Sep  6 14:46:32 2024 +0000, Piotr Caban wrote:
> It's a useful trace, please change it to something like: `if(!once++)
> FIXME(...) else TRACE(...)`.

Done

[Aida Jonikienė (＠DodoGTA)]

On Fri Sep  6 14:47:06 2024 +0000, Piotr Caban wrote:
> void __thiscall _StructuredTaskCollection_dtor(_StructuredTaskCollection *this)
> {
>     TRACE("(%p)\n", this);
>     if (this->count && !__uncaught_exception()) {
>         missing_wait e;
>         WARN("missing call to _RunAndWait\n");
>         missing_wait_ctor_str(&e, "Missing call to _RunAndWait");
>
> The function looks implemented. AFAIR it was not really tested what
> happens in error case - hence the WARN message.

Done

[Alfred Agrell (＠Alcaro)]

Alfred Agrell (@Alcaro) commented about dlls/msvcrt/concurrency.c:
> /* ?_Yield@_Context@details@Concurrency@@SAXXZ */
>  void __cdecl Context_Yield(void)
>  {
> - FIXME("()\n");
> - static unsigned int once;
> -
> - if (!once++)

This function looks threading related, so that's a race condition. Probably harmless in practice, but that kinda stuff makes me nervous. (And it'll print again if it overflows.)

I recommend changing to InterlockedOr.

[Piotr Caban (＠piotr)]

On Fri Sep  6 14:54:24 2024 +0000, Alfred Agrell wrote:
> This function looks threading related, so that's a race condition.
> Probably harmless in practice, but that kinda stuff makes me nervous.
> (And it'll print again if it overflows.)
> I recommend changing to InterlockedOr.

I don't think we should worry about that, it's just a fixme message.

[PATCH v2 0/3] MR6454: msvcrt: Some FIXME once stuff. - approved

[Piotr Caban (＠piotr)]

This merge request was approved by Piotr Caban.