[Mono-dev] [PATCH 06/12] Handle lack of SA_SIGINFO

Miguel de Icaza miguel at novell.com
Fri Mar 26 12:15:11 EDT 2010


Hello,

     In general, if we have code that belongs in a function, it should
be in a function, not hidden as a macro.

     I was going to raise this issue recently with some
MONO_TLS_FOO_BAR that was committed, but figured "This is not likely
going to happen every day".    But since it seems like it is happening
every day, we need to move away from this.   Uppercase defines should
be constants, and only with a few exceptions be bits of code.

Miguel

On Fri, Mar 26, 2010 at 9:29 AM, Andreas Färber <andreas.faerber at web.de> wrote:
> SA_SIGINFO-style signals are part of the optional POSIX XSI (formerly
> Real Time Signals, RTS) feature. Haiku does not implement it (yet) and
> goes so far as to not define SA_SIGINFO to indicate lack thereof.
>
> In mini, there's MONO_ARCH_USE_SIGACTION for this but it doesn't cover
> all uses of SA_SIGINFO; deal with the theoretical case of SA_SIGINFO being
> defined but MONO_ARCH_USE_SIGACTION undefined.
>
> Fix typo in comment.
>
> v2 -> v3:
> * Rework signature macros to match mini.
>
> v1 -> v2:
> * Introduce helper macros, suggested by Paolo Molaro.
>
> This commit is licensed under the MIT X11 license.
> ---
>  mono/metadata/ChangeLog      |    6 ++++++
>  mono/metadata/console-unix.c |   36 ++++++++++++++++++++++++++----------
>  mono/mini/ChangeLog          |    6 ++++++
>  mono/mini/mini-posix.c       |   15 +++++++++++++--
>  4 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/mono/metadata/ChangeLog b/mono/metadata/ChangeLog
> index 243cc51..4c8dd51 100644
> --- a/mono/metadata/ChangeLog
> +++ b/mono/metadata/ChangeLog
> @@ -1,3 +1,9 @@
> +2010-03-26  Andreas Färber  <andreas.faerber at web.de>
> +
> +       * console-unix.c (sigcont_handler, sigwinch_handler): Don't assume
> +       SA_SIGINFO-style signals, fix the build on platforms without (Haiku).
> +
> +       Code is contributed under MIT/X11 license.
>
>  Fri Mar 26 11:33:17 CET 2010 Paolo Molaro <lupus at ximian.com>
>
> diff --git a/mono/metadata/console-unix.c b/mono/metadata/console-unix.c
> index 99e0754..7280b6f 100644
> --- a/mono/metadata/console-unix.c
> +++ b/mono/metadata/console-unix.c
> @@ -262,10 +262,32 @@ sigint_handler (int signo)
>        in_sigint = FALSE;
>  }
>
> +#define SIGHANDLER_VALID(sigh) \
> +       ((sigh) != NULL && \
> +        (sigh) != (void *)SIG_DFL && \
> +        (sigh) != (void *)SIG_IGN)
> +
> +#ifdef SA_SIGINFO
> +#define SIG_HANDLER_SIGNATURE(ftn) ftn (int signo, void *the_siginfo, void *data)
> +#define INVOKE_SIGHANDLER_IF_VALID(siga) G_STMT_START \
> +               if ((siga).sa_flags & SA_SIGINFO) { \
> +                       if (SIGHANDLER_VALID((siga).sa_sigaction)) \
> +                               (*(siga).sa_sigaction) (signo, the_siginfo, data); \
> +               } else if (SIGHANDLER_VALID((siga).sa_handler)) \
> +                       (*(siga).sa_handler) (signo); \
> +       G_STMT_END
> +#else
> +#define SIG_HANDLER_SIGNATURE(ftn) ftn (int signo)
> +#define INVOKE_SIGHANDLER_IF_VALID(siga) G_STMT_START \
> +               if (SIGHANDLER_VALID((siga).sa_handler)) \
> +                       (*(siga).sa_handler) (signo); \
> +       G_STMT_END
> +#endif
> +
>  static struct sigaction save_sigcont, save_sigint, save_sigwinch;
>
>  static void
> -sigcont_handler (int signo, void *the_siginfo, void *data)
> +SIG_HANDLER_SIGNATURE (sigcont_handler)
>  {
>        // Ignore error, there is not much we can do in the sigcont handler.
>        tcsetattr (STDIN_FILENO, TCSANOW, &mono_attr);
> @@ -274,24 +296,18 @@ sigcont_handler (int signo, void *the_siginfo, void *data)
>                write (STDOUT_FILENO, keypad_xmit_str, strlen (keypad_xmit_str));
>
>        // Call previous handler
> -       if (save_sigcont.sa_sigaction != NULL &&
> -           save_sigcont.sa_sigaction != (void *)SIG_DFL &&
> -           save_sigcont.sa_sigaction != (void *)SIG_IGN)
> -               (*save_sigcont.sa_sigaction) (signo, the_siginfo, data);
> +       INVOKE_SIGHANDLER_IF_VALID (save_sigcont);
>  }
>
>  static void
> -sigwinch_handler (int signo, void *the_siginfo, void *data)
> +SIG_HANDLER_SIGNATURE (sigwinch_handler)
>  {
>        int dims = terminal_get_dimensions ();
>        if (dims != -1)
>                cols_and_lines = dims;
>
>        // Call previous handler
> -       if (save_sigwinch.sa_sigaction != NULL &&
> -           save_sigwinch.sa_sigaction != (void *)SIG_DFL &&
> -           save_sigwinch.sa_sigaction != (void *)SIG_IGN)
> -               (*save_sigwinch.sa_sigaction) (signo, the_siginfo, data);
> +       INVOKE_SIGHANDLER_IF_VALID (save_sigwinch);
>  }
>
>  void
> diff --git a/mono/mini/ChangeLog b/mono/mini/ChangeLog
> index 6bea591..fe45481 100755
> --- a/mono/mini/ChangeLog
> +++ b/mono/mini/ChangeLog
> @@ -1,5 +1,11 @@
>  2010-03-26  Andreas Faerber  <andreas.faerber at web.de>
>
> +       * mini-posix.c: Fix the build on platforms without SA_SIGINFO (Haiku).
> +
> +       Code is contributed under MIT/X11 license.
> +
> +2010-03-26  Andreas Faerber  <andreas.faerber at web.de>
> +
>        * mini-x86.c (mono_arch_is_single_step_event,
>        mono_arch_is_breakpoint_event): New define HAVE_SIG_INFO,
>        restrict it to MONO_ARCH_USE_SIGACTION to fix build on Haiku.
> diff --git a/mono/mini/mini-posix.c b/mono/mini/mini-posix.c
> index 823d93b..f53a850 100644
> --- a/mono/mini/mini-posix.c
> +++ b/mono/mini/mini-posix.c
> @@ -82,13 +82,17 @@ save_old_signal_handler (int signo, struct sigaction *old_action)
>        mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_CONFIG,
>                                "Saving old signal handler for signal %d.", signo);
>
> +#ifdef SA_SIGINFO
>        if (! (old_action->sa_flags & SA_SIGINFO)) {
> +#endif
>                handler_to_save->sa_handler = old_action->sa_handler;
> +#ifdef SA_SIGINFO
>        } else {
>  #ifdef MONO_ARCH_USE_SIGACTION
>                handler_to_save->sa_sigaction = old_action->sa_sigaction;
>  #endif /* MONO_ARCH_USE_SIGACTION */
>        }
> +#endif
>        handler_to_save->sa_mask = old_action->sa_mask;
>        handler_to_save->sa_flags = old_action->sa_flags;
>
> @@ -129,13 +133,17 @@ SIG_HANDLER_SIGNATURE (mono_chain_signal)
>        GET_CONTEXT;
>
>        if (saved_handler) {
> +#ifdef SA_SIGINFO
>                if (!(saved_handler->sa_flags & SA_SIGINFO)) {
> +#endif
>                        saved_handler->sa_handler (signal);
> +#ifdef SA_SIGINFO
>                } else {
>  #ifdef MONO_ARCH_USE_SIGACTION
>                        saved_handler->sa_sigaction (signal, info, ctx);
>  #endif /* MONO_ARCH_USE_SIGACTION */
>                }
> +#endif
>                return TRUE;
>        }
>        return FALSE;
> @@ -381,9 +389,12 @@ add_signal_handler (int signo, gpointer handler)
>        g_assert (sigaction (signo, &sa, &previous_sa) != -1);
>
>        /* if there was already a handler in place for this signal, store it */
> -       if (! (previous_sa.sa_flags & SA_SIGINFO) &&
> +       if (
> +#ifdef SA_SIGINFO
> +               ! (previous_sa.sa_flags & SA_SIGINFO) &&
> +#endif
>                        (SIG_DFL == previous_sa.sa_handler)) {
> -               /* it there is no sa_sigaction function and the sa_handler is default, we can safely ignore this */
> +               /* if there is no sa_sigaction function and the sa_handler is default, we can safely ignore this */
>        } else {
>                if (mono_do_signal_chaining)
>                        save_old_signal_handler (signo, &previous_sa);
> --
> 1.6.5.3
>
>
> _______________________________________________
> Mono-devel-list mailing list
> Mono-devel-list at lists.ximian.com
> http://lists.ximian.com/mailman/listinfo/mono-devel-list
>
>


More information about the Mono-devel-list mailing list