diff mbox series

[2/5] Revert "test/alarm: disable bad time cases on Windows"

Message ID 20240808194756.167664-3-stephen@networkplumber.org (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series alarm related patches |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Stephen Hemminger Aug. 8, 2024, 7:46 p.m. UTC 
This reverts commit a089d320338d708f5b7126dab5fd6861c82e6347.

Windows EAL should have been fixed rather than papering over
the bug.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 app/test/test_alarm.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Dmitry Kozlyuk Aug. 9, 2024, 7:23 a.m. UTC | #1 
2024-08-08 12:46 (UTC-0700), Stephen Hemminger:
> This reverts commit a089d320338d708f5b7126dab5fd6861c82e6347.
> 
> Windows EAL should have been fixed rather than papering over
> the bug.

Linux and FreeBSD alarm implementations use the same approach
that limits possible timeout range in API.
Test cases in question check that these values are rejected.
Windows EAL can accept any values.
I think the proper fix would be documenting Unix limitations
at API level (it never worked the other way, so no real breakage),
then adding the same checks to Windows EAL only for consistency.
Dmitry Kozlyuk Aug. 9, 2024, 7:33 a.m. UTC | #2 
2024-08-09 10:23 (UTC+0300), Dmitry Kozlyuk:
> 2024-08-08 12:46 (UTC-0700), Stephen Hemminger:
> > This reverts commit a089d320338d708f5b7126dab5fd6861c82e6347.
> > 
> > Windows EAL should have been fixed rather than papering over
> > the bug.  
> 
> Linux and FreeBSD alarm implementations use the same approach
> that limits possible timeout range in API.
> Test cases in question check that these values are rejected.
> Windows EAL can accept any values.
> I think the proper fix would be documenting Unix limitations
> at API level (it never worked the other way, so no real breakage),
> then adding the same checks to Windows EAL only for consistency.

Oops, I got patch 1/5 in email later somehow. It is doing just that.
Sorry for the noise.
Stephen Hemminger Aug. 9, 2024, 2:59 p.m. UTC | #3 
On Fri, 9 Aug 2024 10:23:24 +0300
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com> wrote:

> 2024-08-08 12:46 (UTC-0700), Stephen Hemminger:
> > This reverts commit a089d320338d708f5b7126dab5fd6861c82e6347.
> > 
> > Windows EAL should have been fixed rather than papering over
> > the bug.  
> 
> Linux and FreeBSD alarm implementations use the same approach
> that limits possible timeout range in API.
> Test cases in question check that these values are rejected.
> Windows EAL can accept any values.

That was a bug. There should not be different behavior
on different OS.

> I think the proper fix would be documenting Unix limitations
> at API level (it never worked the other way, so no real breakage),
> then adding the same checks to Windows EAL only for consistency.

No. All EAL should behave the same.
diff mbox series

Patch

diff --git a/app/test/test_alarm.c b/app/test/test_alarm.c
index 70e97a3109..b4034339b8 100644
--- a/app/test/test_alarm.c
+++ b/app/test/test_alarm.c
@@ -10,7 +10,6 @@ 
 
 #include "test.h"
 
-#ifndef RTE_EXEC_ENV_WINDOWS
 static volatile int flag;
 
 static void
@@ -19,7 +18,6 @@  test_alarm_callback(void *cb_arg)
 	flag = 1;
 	printf("Callback setting flag - OK. [cb_arg = %p]\n", cb_arg);
 }
-#endif
 
 static int
 test_alarm(void)
@@ -29,7 +27,6 @@  test_alarm(void)
 	return 0;
 #endif
 
-#ifndef RTE_EXEC_ENV_WINDOWS
 	/* check if it will fail to set alarm with wrong us value */
 	printf("check if it will fail to set alarm with wrong ms values\n");
 	if (rte_eal_alarm_set(0, test_alarm_callback,
@@ -42,7 +39,6 @@  test_alarm(void)
 		printf("should not be successful with (UINT64_MAX-1) us value\n");
 		return -1;
 	}
-#endif
 
 	/* check if it will fail to set alarm with null callback parameter */
 	printf("check if it will fail to set alarm with null callback parameter\n");