Re: [PATCH] cmd: Add basic handling of Ctrl-C

[Nikolay Sivov]

On 26.03.2015 14:32, Hugh McMaster wrote:
> - /* Enable handling of Ctrl-C signals */
> - if (SetConsoleCtrlHandler((PHANDLER_ROUTINE)console_ctrl_handler, TRUE))
> -
>  WINE_TRACE("cmd: Ctrl handler was installed.\n");
> - else
> -
>  WINE_ERR("cmd: Unable to install Ctrl handler.\n");
> -

Judging by the fact that you needed a cast, that's not right.

[PATCH] cmd: Add basic handling of Ctrl-C

[Hugh McMaster]

On Thu, 26 Mar 2015 14:39:12 +0300, Nikolay Sivov wrote:
> On 26.03.2015 14:32, Hugh McMaster wrote:
> > - /* Enable handling of Ctrl-C signals */
> > - if (SetConsoleCtrlHandler((PHANDLER_ROUTINE)console_ctrl_handler, TRUE))
> > -
> >  WINE_TRACE("cmd: Ctrl handler was installed.\n");
> > - else
> > -
> >  WINE_ERR("cmd: Unable to install Ctrl handler.\n");
> > -
>
> Judging by the fact that you needed a cast, that's not right.

The PHANDLER_ROUTINE is a pointer to the function handler. MSDN's own example [1] on installing a Ctrl handler uses a cast.

Otherwise, what would you suggest?

[1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms685049%28v=vs.85%...

[PATCH] cmd: Add basic handling of Ctrl-C

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2015-03-26 um 13:46 schrieb Hugh McMaster:
> The PHANDLER_ROUTINE is a pointer to the function handler. MSDN's 
> own example [1] on installing a Ctrl handler uses a cast.
>
> Otherwise, what would you suggest?

If the function signature (including calling convention) matches the
one SetConsoleCtrlHandler expects the compiler should accept it
without a cast and without a warning.

In your patch you should probably use WINAPI for console_ctrl_handler.

That the MSDN example uses a cast is just terrible. If you get e.g.
the calling convention wrong you're begging for random crashes or
incorrectly passed parameters that are difficult to debug.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVFBSFAAoJEN0/YqbEcdMwJ94P/jJRST2oGaIkTasgTe8Vf9NV
slfdp/vvk+y+hxzAmZrFK6wd76Bq+tUctnU6FY+KLZEYXocMJxTviSKuDsNGUvUi
4U2oHgFBWLys6tXiBfDHSHnjKezTL3yRm0ZCYvZKYgeQBQusCpNIpRETqntjbZg8
egeSF+l2vlSLNC/O2XnzQfXzjsc/uOMdcT9G1Tom3Y/dCzHRW+NfET4l1uGX4G+6
JcZKeYqKvcHo6T0EhP+6pagMZh17l2sc43sVWtZwuXRYQwRQJdNDVl/mvlJziqnJ
vbyfQxjyqO7KMppIqwSP+tpC5Z5wbBWdx14+NXKa82APi39dNtzgdR3eyN4hUMCP
D/8OuLuAJvg60FYj7Jb17eLs9l7n9ufXT+BcVe2tb/F8G+fhVswA5cMqX1LjAN9G
VIU5uiEWEOgfHa+joDuk/yQhy78+vnbC1H3ihabvA0CNnyms4DE/if7hL6JslgKY
qUcRLYZSrj0Ikql/ThGSn+/SNPEcFqb0kEI2jmND+kpcdKKl7fJ+ueecuO1Oa9fT
/gSRBzETzlaonGpi+JiMbYYNKbUsUzn6dBs6djB+unvki5LG8TsRxc8l5Mwh6eHB
ABHfO5EOCEDYgJDY1l9f5qozCHt/beiKrzSO3An2+FJ7cjP6Txvng9a04d4NzXyr
YGYKcHZW/MTAGl3iDD1d
=H+J+
-----END PGP SIGNATURE-----

[PATCH] cmd: Add basic handling of Ctrl-C

[Hugh McMaster]

On Thu, 26 Mar 2015 15:15:34 +0100, Stefan Dösinger wrote:
> Am 2015-03-26 um 13:46 schrieb Hugh McMaster:
> > The PHANDLER_ROUTINE is a pointer to the function handler. MSDN's 
> > own example [1] on installing a Ctrl handler uses a cast.
> >
> > Otherwise, what would you suggest?
>
> If the function signature (including calling convention) matches the
> one SetConsoleCtrlHandler expects the compiler should accept it
> without a cast and without a warning.
>
> In your patch you should probably use WINAPI for console_ctrl_handler.

The compiler accepted "BOOL WINAPI console_ctrl_handler" without warnings.

Thanks. I'll send a new patch.