[FFmpeg-devel] [PATCH 1/2] ffmpeg: reset tty state correctly

Nicolas George george at nsup.org
Mon Jul 27 10:39:16 CEST 2015


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 <gajjanagadde at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150727/f5d8e329/attachment.sig>


More information about the ffmpeg-devel mailing list