[PATCH 0/1] Patch removing some dead code

[David Kahurani]

The following patch removes some dead code. It is not clear to me
whether the FIXME should remain as it is so it has been left out.

David Kahurani (1):
  dlls/ntdll: Remove dead code

dlls/ntdll/unix/system.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

[PATCH 1/1] dlls/ntdll: Remove dead code

[David Kahurani]

The if clause here does not make much sense and
would probably get optimized out by a smart compiler

Signed-off-by: David Kahurani 
---
 dlls/ntdll/unix/system.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c
index 82c9f0d..88050fb 100644
--- a/dlls/ntdll/unix/system.c
+++ b/dlls/ntdll/unix/system.c
@@ -2399,7 +2399,6 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class,
     case SystemPerformanceInformation:  /* 2 */
     {
         SYSTEM_PERFORMANCE_INFORMATION spi;
-        static BOOL fixme_written = FALSE;

get_performance_info( &spi );
         len = sizeof(spi);
@@ -2409,10 +2408,7 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class,
             else memcpy( info, &spi, len);
         }
         else ret = STATUS_INFO_LENGTH_MISMATCH;
-        if(!fixme_written) {
-            FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
-            fixme_written = TRUE;
-        }
+        FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
         break;
     }

[PATCH 1/1] dlls/ntdll: Remove dead code

[Eric Pouech]

Le 06/10/2021 à 16:01, David Kahurani a écrit :
> The if clause here does not make much sense and
> would probably get optimized out by a smart compiler
> Signed-off-by: David Kahurani
>
> dlls/ntdll/unix/system.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c
> index 82c9f0d..88050fb 100644
> --- a/dlls/ntdll/unix/system.c
> +++ b/dlls/ntdll/unix/system.c
> @@ -2399,7 +2399,6 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class,
>       case SystemPerformanceInformation:  /* 2 */
>       {
>           SYSTEM_PERFORMANCE_INFORMATION spi;
> -
>    static BOOL fixme_written = FALSE;
>
>     get_performance_info( &spi );
>     len = sizeof(spi);
>
> @@ -2409,10 +2408,7 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class,
>               else memcpy( info, &spi, len);
>           }
>           else ret = STATUS_INFO_LENGTH_MISMATCH;
> -
>    if(!fixme_written) {
> -
>        FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
> -
>        fixme_written = TRUE;
> -
>    }
> -
>    FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
>     break;
> }

storage of fixme_written is static... so the FIXME is only printed once 
if that code is called several times

should be kept as it is

+A

[PATCH 1/1] dlls/ntdll: Remove dead code

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=99562

Your paranoid android.

=== debiant2 (build log) ===

error: patch failed: dlls/ntdll/unix/system.c:2399
Task: Patch failed to apply

=== debiant2 (build log) ===

error: patch failed: dlls/ntdll/unix/system.c:2399
Task: Patch failed to apply

[PATCH 1/1] dlls/ntdll: Remove dead code

[Paul Gofman]

On 10/6/21 17:01, David Kahurani wrote:
> The if clause here does not make much sense and
> would probably get optimized out by a smart compiler

Can you please explain why it would be optimized out, exactly? if clause 
does make some sense, it makes fixme: displayed just once per process 
instead of spamming it great amount of times.
> Signed-off-by: David Kahurani
>
> dlls/ntdll/unix/system.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c
> index 82c9f0d..88050fb 100644
> --- a/dlls/ntdll/unix/system.c
> +++ b/dlls/ntdll/unix/system.c
> @@ -2399,7 +2399,6 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class,
>       case SystemPerformanceInformation:  /* 2 */
>       {
>           SYSTEM_PERFORMANCE_INFORMATION spi;
> -
>    static BOOL fixme_written = FALSE;
>
>     get_performance_info( &spi );
>     len = sizeof(spi);
>
> @@ -2409,10 +2408,7 @@ NTSTATUS WINAPI NtQuerySystemInformation( SYSTEM_INFORMATION_CLASS class,
>               else memcpy( info, &spi, len);
>           }
>           else ret = STATUS_INFO_LENGTH_MISMATCH;
> -
>    if(!fixme_written) {
> -
>        FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
> -
>        fixme_written = TRUE;
> -
>    }
> -
>    FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
>     break;
> }

[PATCH 1/1] dlls/ntdll: Remove dead code

[David K.]

On Wed, Oct 6, 2021 at 5:12 PM Paul Gofman  wrote:
> On 10/6/21 17:01, David Kahurani wrote:
> > The if clause here does not make much sense and
> > would probably get optimized out by a smart compiler
>
> Can you please explain why it would be optimized out, exactly? if clause
> does make some sense, it makes fixme: displayed just once per process
> instead of spamming it great amount of times.

Didn't take into consideration the storage specifier, sorry and thanks for
pointing that out.

Without the storage classifier static this would have been dead code. In
other words, code always gives the same result in which case a compiler
would replace it with the result instead.
> > Signed-off-by: David Kahurani
> >
> > dlls/ntdll/unix/system.c | 6 +-----
> >   1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c
> > index 82c9f0d..88050fb 100644
> > --- a/dlls/ntdll/unix/system.c
> > +++ b/dlls/ntdll/unix/system.c
> > @@ -2399,7 +2399,6 @@ NTSTATUS WINAPI NtQuerySystemInformation(
>
> SYSTEM_INFORMATION_CLASS class,
> >   case SystemPerformanceInformation:  /* 2 */
> >   {
> >       SYSTEM_PERFORMANCE_INFORMATION spi;
> > -
> >    static BOOL fixme_written = FALSE;
> >
> >     get_performance_info( &spi );
> >     len = sizeof(spi);
> >
> > @@ -2409,10 +2408,7 @@ NTSTATUS WINAPI NtQuerySystemInformation(
>
> SYSTEM_INFORMATION_CLASS class,
> >           else memcpy( info, &spi, len);
> >       }
> >       else ret = STATUS_INFO_LENGTH_MISMATCH;
> > -
> >    if(!fixme_written) {
> > -
> >        FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
> > -
> >        fixme_written = TRUE;
> > -
> >    }
> > -
> >    FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
> >     break;
> > }

[PATCH 1/1] dlls/ntdll: Remove dead code

[David K.]

This trick only occurs once. Haven't seen this anywhere else in the code.
Why this specific function?

Don't you people suppose other code might benefit from this?

On Wed, Oct 6, 2021 at 5:24 PM David K.  wrote:
> On Wed, Oct 6, 2021 at 5:12 PM Paul Gofman 
> wrote:
> > On 10/6/21 17:01, David Kahurani wrote:
> > > The if clause here does not make much sense and
> > > would probably get optimized out by a smart compiler
> >
> > Can you please explain why it would be optimized out, exactly? if clause
> > does make some sense, it makes fixme: displayed just once per process
> > instead of spamming it great amount of times.
>
> Didn't take into consideration the storage specifier, sorry and thanks for
> pointing that out.
>
> Without the storage classifier static this would have been dead code. In
> other words, code always gives the same result in which case a compiler
> would replace it with the result instead.
> > > Signed-off-by: David Kahurani
> > >
> > > dlls/ntdll/unix/system.c | 6 +-----
> > >   1 file changed, 1 insertion(+), 5 deletions(-)
> > >
> > > diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c
> > > index 82c9f0d..88050fb 100644
> > > --- a/dlls/ntdll/unix/system.c
> > > +++ b/dlls/ntdll/unix/system.c
> > > @@ -2399,7 +2399,6 @@ NTSTATUS WINAPI NtQuerySystemInformation(
> >
> > SYSTEM_INFORMATION_CLASS class,
> > >   case SystemPerformanceInformation:  /* 2 */
> > >   {
> > >       SYSTEM_PERFORMANCE_INFORMATION spi;
> > > -
> > >    static BOOL fixme_written = FALSE;
> > >
> > >     get_performance_info( &spi );
> > >     len = sizeof(spi);
> > >
> > > @@ -2409,10 +2408,7 @@ NTSTATUS WINAPI NtQuerySystemInformation(
> >
> > SYSTEM_INFORMATION_CLASS class,
> > >           else memcpy( info, &spi, len);
> > >       }
> > >       else ret = STATUS_INFO_LENGTH_MISMATCH;
> > > -
> > >    if(!fixme_written) {
> > > -
> > >        FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
> > > -
> > >        fixme_written = TRUE;
> > > -
> > >    }
> > > -
> > >    FIXME("info_class SYSTEM_PERFORMANCE_INFORMATION\n");
> > >     break;
> > > }

[PATCH 1/1] dlls/ntdll: Remove dead code

[Zebediah Figura (she/her)]

On 10/6/21 10:45, David K. wrote:
> This trick only occurs once. Haven't seen this anywhere else in the code.
> Why this specific function?
>
> Don't you people suppose other code might benefit from this?

It exists in plenty of other places. A simple grep for 'fixme_written' 
yields another result, and the more usual 'static (unsigned) int once' 
yields many.

(Perhaps we should borrow vkd3d's FIXME_ONCE macro...)

[PATCH 1/1] dlls/ntdll: Remove dead code

[David K.]

On Wed, Oct 6, 2021 at 7:09 PM Zebediah Figura (she/her) <
[e-mail]> wrote:
> On 10/6/21 10:45, David K. wrote:
> > This trick only occurs once. Haven't seen this anywhere else in the code.
> > Why this specific function?
> >
> > Don't you people suppose other code might benefit from this?
>
> It exists in plenty of other places. A simple grep for 'fixme_written'
> yields another result, and the more usual 'static (unsigned) int once'
> yields many.

Definitely would be great, at least for use with new code.
> (Perhaps we should borrow vkd3d's FIXME_ONCE macro...)