Re: A Valgrind-observant ncurses

[Bill Gray]

Hi Branden,  all,

On 6/20/24 18:06, G. Branden Robinson wrote:
> Doesn't printw() update curscr (via waddstr()), so by the time of the
> next curses function call, it would no longer require this buffer?

   It looks as if the buffer is re-used each time you call printw(). 
Call it once,  and the buffer is allocated;  call it again,  and it's 
re-used (if it's already large enough) or resized (if it isn't).  This 
logic is all encapsulated in 'safe_sprintf.c',  where the buffer is 
'my_buffer' and its size is 'my_length'.

   With your modification,  the buffer is allocated and used on the 
first call,  but then immediately freed.  On the second call,  we 
attempt to use the (now freed) pointer,  or to resize it.  Either way, 
we'll crash.

   I think what has to happen instead is that,  at either endwin() or 
delscreen(),  we call _nc_printf_string( NULL,  NULL).  If you look down 
near the end of that function,  you can see that those arguments are the 
magic values to cause 'my_buffer' to be freed,  set to NULL,  and 
'my_length' set to zero.  If you _do_ restart curses,  the buffer may 
get allocated again.

   As Thomas notes,  endwin() is limited in what it can free,  because 
you may restart curses.  To get Valgrind to report only five or six 
unfreed allocations,  you have to use newterm() and,  at shutdown,  pass 
the resulting SCREEN pointer to delscreen().

-- Bill

> Testing my own hypothesis:
>
> $ git di ncurses
> diff --git a/ncurses/base/lib_printw.c b/ncurses/base/lib_printw.c
> index d901b727a..2cf87f887 100644
> --- a/ncurses/base/lib_printw.c
> +++ b/ncurses/base/lib_printw.c
> @@ -162,6 +162,7 @@ vw_printw(WINDOW *win, const char *fmt, va_list argp)
>       buf = NCURSES_SP_NAME(_nc_printf_string) (NCURSES_SP_ARGx fmt, argp);
>       if (buf != 0) {
>          code = waddstr(win, buf);
> +       free(buf);
>       }
>       returnCode(code);
>   }
>
> Well, nope.  ./test/bs silently dies with status 1 if I add that line.
>
> I'm a little puzzled as to why.
>
> Here's the backtrace:
>
> #0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
> #1  0x00007ffff7d73537 in __GI_abort () at abort.c:79
> #2  0x00007ffff7dcb3e8 in __libc_message (action=action@entry=do_abort, 
> fmt=fmt@entry=0x7ffff7ee9390 "%s\n")
>      at ../sysdeps/posix/libc_fatal.c:155
> #3  0x00007ffff7dd26da in malloc_printerr (str=str@entry=0x7ffff7eeb730 "double free 
> or corruption (top)")
>      at malloc.c:5347
> #4  0x00007ffff7dd3ce4 in _int_free (av=0x7ffff7f1fb80 <main_arena>, 
> p=0x5555555ceb80, have_lock=<optimized out>)
>      at malloc.c:4309
> #5  0x00007ffff7f732ef in vw_printw (win=0x5555555ab7e0,
>      fmt=0x55555555a0c0 "To position your ships: move the cursor to a spot, 
> then", argp=0x7fffffffda70)
>      at ../ncurses/./base/lib_printw.c:165
> #6  0x00007ffff7f73122 in mvprintw (y=15, x=0,
>      fmt=0x55555555a0c0 "To position your ships: move the cursor to a spot, 
> then")
>      at ../ncurses/./base/lib_printw.c:104
> #7  0x0000555555556f7c in initgame () at ../test/bs.c:411
> #8  0x000055555555977e in main (argc=1, argv=0x7fffffffdcb8) at 
> ../test/bs.c:1251
> ##(gdb) f 5
> #5  0x00007ffff7f732ef in vw_printw (win=0x5555555ab7e0,
>      fmt=0x55555555a0c0 "To position your ships: move the cursor to a spot, 
> then", argp=0x7fffffffda70)
>      at ../ncurses/./base/lib_printw.c:165
> 165             free(buf);
>
> The previous function call in this frame is `waddstr()`, which
> eventually lands in `waddch_nosync()`, which neither allocates nor frees
> memory.  So the faulure is a mystery to me.
>
> Regards,
> Branden