Re: [Nano-devel] [PATCH] restore terminal and print backtrace for segfaults and assertions

[Mike Frysinger]

On 15 Jan 2018 00:59, Brand Huntsman wrote:
> These patches add a segfault handler and custom assertion handler that 
> restore the terminal before exiting. The first patch also displays a 
> backtrace.
> 
> The backtrace-less patch only increased the nano binary by 32 bytes and
> would be nice to have in case nano ever segfaults in normal use or when 
> testing. The backtrace patch would give users an idea where nano crashed, for 
> bug reports, but increased the binary by 24k. The backtrace code could be
> placed behind a --with-backtrace configure option for users or --enable-debug 
> for devs.

it would need to be behind some configure flags or feature test.
you're using linker & C library features that are not portable.

> --- a/src/nano.c
> +++ b/src/nano.c
> @@ -1890,8 +1890,32 @@ void do_output(char *output, size_t output_len, bool 
> allow_cntrls)
>               update_line(openfile->current, openfile->current_x);
>  }
>  
> +void print_backtrace(int skip_to)
> +{
> +     void *callstack[16];
> +     int i, frames = backtrace(callstack, 16);
> +     char **strings = backtrace_symbols(callstack, frames);
> +     for(i = skip_to + 1; i < frames; i++) printf("|  %s\n", strings[i]);

need space after the "for" and unwrap the loop so it isn't on one line

error messages should be sent to stderr, not stdout

> +void segfault_handler(int nothing)

static

> +{
> +     endwin();
> +     tcsetattr(0, TCSANOW, &oldterm);
> +     printf("Segmentation fault\n");

it isn't safe to call printf or any other FILE based functions from a
signal handler.  you have to use write or other async funcs directly.

> --- a/src/nano.h
> +++ b/src/nano.h
>
> -#include <assert.h>
> +#include <execinfo.h>
> +
> +#define assert(expr) if(!(expr)){ \
> +     endwin(); \
> +     tcsetattr(0, TCSANOW, &oldterm); \
> +     printf("Assertion (%s) failed: %s %s() %d\n", #expr, __FILE__, 
> __func__, __LINE__); \
> +     print_backtrace(0); \
> +}

shouldn't clobber the standard assert macro ...
-mike

signature.asc
Description: Digital signature