dlls/kernel/console.c::SetConsoleCtrlHandler() cleanup/fixup
Robert Shearman
R.J.Shearman at warwick.ac.uk
Fri Apr 16 06:37:55 CDT 2004
Peter Riocreux wrote:
>
> "Dimitrie O. Paun" <dpaun at rogers.com> writes:
>
> > On April 15, 2004 7:54 pm, Alexandre Julliard wrote:
> >> 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.
I wrote a test program but I haven't integrated it with the Wine test
framework (attempting to remove the default handler is not practical
anyway).
>
> I figured that *some* value, even guessed-at, was better than none -
> it would allow someone to distinguish between causes of failure. I
> completely agree that the correct values should be used when they are
> available - and hence I have now put WARNs in where the guessed-at
> values are for the moment.
I would agree with you (especially as you have noted that the errors may not
be correct), but many of us have been stuck debugging an application that
depends on a particular error being returned and fails otherwise. I have
commented on the error codes below:
> 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 09:55:24 -0000
> @@ -1448,17 +1448,21 @@
> {
> 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);
This error code is right.
> + WARN("SetLastError() used guessed-at value\n");
> + return FALSE;
> + }
> ch->handler = func;
> RtlEnterCriticalSection(&CONSOLE_CritSect);
> ch->next = CONSOLE_Handlers;
> @@ -1467,34 +1471,39 @@
> }
> else
> {
> - struct ConsoleHandler** ch;
> RtlEnterCriticalSection(&CONSOLE_CritSect);
> - for (ch = &CONSOLE_Handlers; *ch; *ch = (*ch)->next)
> - {
> - if ((*ch)->handler == func) break;
> - }
> - if (*ch)
> - {
> - struct ConsoleHandler* rch = *ch;
> + if (func == CONSOLE_DefaultHandler)
> + {
> + ERR("Attempt to remove default CtrlHandler %\n", func);
> + WARN("SetLastError() used guessed-at value\n");
> + SetLastError(ERROR_FUNCTION_FAILED);
This should be 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);
> + WARN("SetLastError() used guessed-at value\n");
> + SetLastError(ERROR_INVALID_ADDRESS);
This should be ERROR_INVALID_PARAMETER.
> + ret = FALSE;
> + }
> + }
> RtlLeaveCriticalSection(&CONSOLE_CritSect);
> }
> return ret;
Rob
More information about the wine-devel
mailing list