[PATCH 0/1] Patch removing some dead code
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(-) -- 2.33.0
The if clause here does not make much sense and would probably get optimized out by a smart compiler Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- 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; } -- 2.33.0
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<k.kahurani(a)gmail.com> --- 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
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
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 <k.kahurani(a)gmail.com> --- 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; }
On Wed, Oct 6, 2021 at 5:12 PM Paul Gofman <pgofman(a)codeweavers.com> 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 <k.kahurani(a)gmail.com> --- 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; }
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. <k.kahurani(a)gmail.com> wrote:
On Wed, Oct 6, 2021 at 5:12 PM Paul Gofman <pgofman(a)codeweavers.com> 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 <k.kahurani(a)gmail.com> --- 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; }
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...)
On Wed, Oct 6, 2021 at 7:09 PM Zebediah Figura (she/her) < zfigura(a)codeweavers.com> 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...)
participants (6)
-
David K. -
David Kahurani
-
Eric Pouech -
Marvin -
Paul Gofman -
Zebediah Figura (she/her)