RE: dlls/kernel/console.c::SetConsoleCtrlHandler() cleanup/fixup

[Robert Shearman]

Peter Riocreux wrote:
> "Dimitrie O. Paun"  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