dlls/kernel/console.c::SetConsoleCtrlHandler() cleanup/fixup
Peter Riocreux
par+wine_devel at silistix.com
Fri Apr 16 09:25:57 CDT 2004
Alexandre Julliard <julliard at winehq.org> writes:
> Peter Riocreux <par+wine_devel at silistix.com> writes:
>
>> It gives every impression of working for me, but I have only tested it
>> with one application.
>>
>> Suggestions for better extended error codes appreciated.
>
> Error codes should not be based on suggestions, what you should do is
> write a small test program under Windows to find out the real codes.
Robert Shearman provided some info on this and the patch is updated
appropriately.
I have also (thanks to a pointer from Ferenc Wagner) created some test
stuff. The test compiles but is not run yet as I haven't learned my
way around the build process enough to figure out how to make the
tests work. I thought I would run this by people now though so I can
be steered straight if I have deviated from TheOneTrueWay.
I may have completely misunderstood the macros in the test system and
misused them, but it was not obvious (to mine eyes at least) what they
all did.
The big issue is that I cannot work out how to test attempting to
remove the default handler without knowing what real Windows OSes call
both the default handler and the handler list and or having a function
to find them, or a fixed address that they always occur at. As a
result I have #if 0-ed the thing that needs that. Can anyone point me
at anything about this so I can make the patch test those things?
Also, is there a way to make the HeapAlloc fail so I can test that
failure path?
Note also the commented deficiencies of the test.
Peter
Index: dlls/kernel/console.c
===================================================================
RCS file: /home/wine/wine/dlls/kernel/console.c,v
retrieving revision 1.30
diff -u -r1.30 console.c
--- dlls/kernel/console.c 13 Apr 2004 21:16:26 -0000 1.30
+++ dlls/kernel/console.c 16 Apr 2004 14:12:31 -0000
@@ -1448,17 +1448,20 @@
{
BOOL ret = TRUE;
- FIXME("(%p,%i) - no error checking or testing yet\n", func, add);
-
if (!func)
{
+ TRACE("(%p,%i) - first arg being NULL is not allowed in WinME, Win98 or Win95\n", func, add);
CONSOLE_IgnoreCtrlC = add;
}
else if (add)
{
struct ConsoleHandler* ch = HeapAlloc(GetProcessHeap(), 0, sizeof(struct ConsoleHandler));
- if (!ch) return FALSE;
+ if (!ch)
+ {
+ SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+ return FALSE;
+ }
ch->handler = func;
RtlEnterCriticalSection(&CONSOLE_CritSect);
ch->next = CONSOLE_Handlers;
@@ -1467,34 +1470,46 @@
}
else
{
- struct ConsoleHandler** ch;
+ /*
+ ** As CONSOLE_DefaultHandler will always be the last in the
+ ** chain (new handlers are added at the front), should we
+ ** remove any instances of CONSOLE_DefaultHandler except the
+ ** last one when requested? I.e., <philosophy> is any but the
+ ** last *actually* the default handler? </philosophy> When we
+ ** remove a handler, should we remove every instance of it,
+ ** just the first (current behaviour), or just the last?
+ */
RtlEnterCriticalSection(&CONSOLE_CritSect);
- for (ch = &CONSOLE_Handlers; *ch; *ch = (*ch)->next)
- {
- if ((*ch)->handler == func) break;
- }
- if (*ch)
+ if (func == CONSOLE_DefaultHandler)
{
- struct ConsoleHandler* rch = *ch;
+ ERR("Attempt to remove default CtrlHandler %p\n", func);
+ SetLastError(ERROR_INVALID_PARAMETER);
+ ret = FALSE;
+ }
+ else
+ {
+ struct ConsoleHandler* ch, * prev ;
+ prev = NULL;
+ for (ch = CONSOLE_Handlers; ch; prev = ch, ch = ch->next)
+ {
+ if (ch->handler == func)
+ {
+ if (ch == CONSOLE_Handlers)
+ CONSOLE_Handlers = ch->next;
+ else
+ prev->next = ch->next;
+ HeapFree(GetProcessHeap(), 0, ch);
+ break;
+ }
+ }
- /* sanity check */
- if (rch == &CONSOLE_DefaultConsoleHandler)
- {
- ERR("Who's trying to remove default handler???\n");
- ret = FALSE;
- }
- else
- {
- rch = *ch;
- *ch = (*ch)->next;
- HeapFree(GetProcessHeap(), 0, rch);
- }
- }
- else
- {
- WARN("Attempt to remove non-installed CtrlHandler %p\n", func);
- ret = FALSE;
- }
+ if (! ch)
+ {
+ WARN("Attempt to remove non-installed CtrlHandler %p\n", func);
+ SetLastError(ERROR_INVALID_PARAMETER);
+ ret = FALSE;
+ }
+ }
RtlLeaveCriticalSection(&CONSOLE_CritSect);
}
return ret;
Index: dlls/kernel/tests/console.c
===================================================================
RCS file: /home/wine/wine/dlls/kernel/tests/console.c,v
retrieving revision 1.4
diff -u -r1.4 console.c
--- dlls/kernel/tests/console.c 26 Jan 2004 20:23:25 -0000 1.4
+++ dlls/kernel/tests/console.c 16 Apr 2004 14:12:31 -0000
@@ -537,6 +537,56 @@
}
#endif
+/* DEFICIENCIES:
+** - Does not attempt to test whether disabling CtrlC actually has
+** any effect (either by looking at the variable or synthesising an
+ event).
+** - Does not check whether the handler was *actually* installed or
+** removed.
+*/
+static BOOL WINAPI testDummyHandler(DWORD dwCtrlType)
+{
+ ExitProcess(0);
+ /* should never go here */
+ return TRUE;
+}
+
+static void testSetConsoleCtrlHandler(void)
+{
+ ok(SetConsoleCtrlHandler(NULL, FALSE) == TRUE,
+ "SetConsoleCtrlHandler: could not enable CtrlC handling: GetLastError() returned %lu\n",
+ GetLastError());
+
+ ok(SetConsoleCtrlHandler(NULL, TRUE) == TRUE,
+ "SetConsoleCtrlHandler: could not disable CtrlC handling: GetLastError() returned %lu\n",
+ GetLastError());
+
+ ok(SetConsoleCtrlHandler(NULL, FALSE) == TRUE,
+ "SetConsoleCtrlHandler: could not enable CtrlC handling: GetLastError() returned %lu\n",
+ GetLastError());
+
+#if 0
+ ok(SetConsoleCtrlHandler(CONSOLE_DefaultHandler, FALSE) == FALSE,
+ "SetConsoleCtrlHandler: attempt to remove the default handler apparently succeeded!\n");
+ ok(GetLastError() == ERROR_INVALID_PARAMETER,
+ "SetConsoleCtrlHandler: attempt to remove the default handler: GetLastError() should have returned %u, actually %lu\n",
+ ERROR_INVALID_PARAMETER, GetLastError());
+#endif
+
+ ok(SetConsoleCtrlHandler(testDummyHandler, FALSE) == FALSE,
+ "SetConsoleCtrlHandler: attempt to remove a non-existent handler apparently succeeded!\n");
+ ok(GetLastError() == ERROR_INVALID_PARAMETER,
+ "SetConsoleCtrlHandler: attempt to remove a non-existent handler: GetLastError() should have returned %u, actually %lu\n",
+ ERROR_INVALID_PARAMETER, GetLastError());
+
+ ok(SetConsoleCtrlHandler(testDummyHandler, TRUE) == TRUE,
+ "SetConsoleCtrlHandler: attempt to install a handler failed: GetLastError() returned %lu\n",
+ GetLastError());
+ ok(SetConsoleCtrlHandler(testDummyHandler, FALSE) == TRUE,
+ "SetConsoleCtrlHandler: attempt to uninstall a handler failed: GetLastError() returned %lu\n",
+ GetLastError());
+}
+
START_TEST(console)
{
HANDLE hConIn, hConOut;
@@ -579,6 +629,8 @@
/* testScroll(hCon, sbi.dwSize); */
/* will test sb creation / modification... */
/* testScreenBuffer() */
+ /* tests adding and removing handlers for CtrlC etc */
+ testSetConsoleCtrlHandler();
/* still to be done: events generation, access rights & access on objects */
}
More information about the wine-patches
mailing list