Re: A Valgrind-observant ncurses

[Thomas Dickey]

On Thu, Jun 20, 2024 at 07:49:54PM -0400, Bill Gray wrote:
> 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().

right - I do that in one of the test programs (test_delwin.c), and could
do that in others, but am getting enough coverage using the debugging
features via exit_curses and exit_terminfo.

I added exit_curses and exit_terminfo so that developers could call those
functions and plug in the debugging (and leak-freeing) library for
analysis.  For whatever reason (I recall Bram Moolenaar in this case)
some developers don't want to do that, but it works for me.

Anyway - sure, I'll take a look to see if there's some fluff that could
be trimmed between endwin/refresh, but bug reports (such as the configure
script with cygwin) get more attention.
 
> -- 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
> 

-- 
Thomas E. Dickey <dickey@invisible-island.net>
https://invisible-island.net

signature.asc
Description: PGP signature