L'octidi 8 thermidor, an CCXXIII, Ganesh Ajjanagadde a écrit :
> tty state was not being reset upon "hard" signals (SIGSEGV etc)

A good shell can do that for you.

> This resets tty state in such situations, fixes Ticket2964

This ticket is only about tcsetattr() not putting the tty in raw mode when
stderr is redirected. Removing the isatty(2) test should be enough to fix
it, but reviewing the discussions that lead to the test is necessary to know
why it was deemed useful in the first place.

> 
> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com>
> ---
>  ffmpeg.c | 38 ++++++++++++++++++++++++++++----------
>  1 file changed, 28 insertions(+), 10 deletions(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 751c7d3..8b5a705 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -325,10 +325,29 @@ sigterm_handler(int sig)
>      received_sigterm = sig;
>      received_nb_signals++;
>      term_exit_sigsafe();
> -    if(received_nb_signals > 3) {
> -        write(2/*STDERR_FILENO*/, "Received > 3 system signals, hard 
> exiting\n",
> -                           strlen("Received > 3 system signals, hard 
> exiting\n"));
>  

> +    switch (sig) {
> +        /* 2 = STDERR_FILENO */
> +        case SIGSEGV:
> +            write(2, "Segmentation fault, hard exiting\n",
> +              strlen("Segmentation fault, hard exiting\n"));
> +            abort();
> +        case SIGILL:
> +            write(2, "Invalid instruction, hard exiting\n",
> +              strlen("Invalid instruction, hard exiting\n"));
> +            abort();
> +        case SIGFPE:
> +            write(2, "Arithmetic exception, hard exiting\n",
> +              strlen("Arithmetic exception, hard exiting\n"));
> +            abort();
> +            break;
> +        default:
> +            break;
> +    }

That lakes a lot of code duplication.

    char *msg = NULL;

    switch (sig) {
        case X: msg = "signal X"; break;
        ...
    }
    if (msg)
        write(2, msg, strlen(msg));

But this change is not specified in the commit message.

> +
> +    if(received_nb_signals > 3) {
> +        write(2, "Received > 3 system signals, hard exiting\n",
> +          strlen("Received > 3 system signals, hard exiting\n"));
>          exit(123);
>      }
>  }
> @@ -370,11 +389,7 @@ void term_init(void)
>  #if HAVE_TERMIOS_H
>      if(!run_as_daemon){
>          struct termios tty;

> -        int istty = 1;
> -#if HAVE_ISATTY
> -        istty = isatty(0) && isatty(2);
> -#endif
> -        if (istty && tcgetattr (0, &tty) == 0) {
> +        if (tcgetattr (0, &tty) == 0) {

Why?

>              oldtty = tty;
>              restore_tty = 1;
>  
> @@ -393,8 +408,11 @@ void term_init(void)
>      }
>  #endif
>  
> -    signal(SIGINT , sigterm_handler); /* Interrupt (ANSI).    */
> -    signal(SIGTERM, sigterm_handler); /* Termination (ANSI).  */
> +    signal(SIGINT , sigterm_handler); /* Interrupt (ANSI).              */
> +    signal(SIGTERM, sigterm_handler); /* Termination (ANSI).            */

> +    signal(SIGSEGV, sigterm_handler); /* Segmentation fault (ANSI).     */
> +    signal(SIGILL , sigterm_handler); /* Invalid instruction (ANSI).    */
> +    signal(SIGFPE , sigterm_handler); /* Arithmetic error (ANSI).       */

NO!!!

When a crash happens, we want it to happen right there, possibly leave a
core. We do not want a signal handler to mess up the remains.

>  #ifdef SIGXCPU
>      signal(SIGXCPU, sigterm_handler);
>  #endif

Attachment: signature.asc
Description: Digital signature

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to