lib/56174: libcurses: __makewin() accesses uninitialized WINDOW field

[mforney%mforney.org@localhost]

>Number:         56174
>Category:       lib
>Synopsis:       libcurses: __makewin() accesses uninitialized WINDOW field
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat May 15 07:10:01 +0000 2021
>Originator:     Michael Forney
>Release:        trunk
>Organization:
>Environment:
>Description:
I noticed the following warning when compiling libcurses/newwin.c:

In file included from libcurses/newwin.c:37:
libcurses/newwin.c: In function '__makenew':
libcurses/curses_private.h:89:29: warning: '*win.battr' may be used uninitialized [-Wmaybe-uninitialized]
   89 |         ((c).battr) = ((((c).battr) & WA_ATTRIBUTES ) | ((w) << WCW_SHIFT )); \
      |                         ~~~~^~~~~~~
libcurses/newwin.c:384:9: note: in expansion of macro 'SET_BGWCOL'
  384 |         SET_BGWCOL(*win, 1);
      |         ^~~~~~~~~~

I believe this warning is legitimate. The __makenew() function allocates the win structure, and then invokes the SET_BGWCOL macro without previously setting battr.

However, it looks like both callers of __makenew() overwrite the previous value of battr themselves. Additionally, I found no uses of the BGWCOL macro in the libcurses codebase.
>How-To-Repeat:
1. Compile libcurses/newwin.c with -Wall -O2.
2. Observe warning about use of uninitialized field.
>Fix:
If BGWCOL should be set on windows created with __newwin and __subwin, then __makenew() should initialize battr to 0 before invoking SET_BGWCOL, and __newwin/__subwin should take care to preserve it when setting battr.

I am not sure if this fix is correct, but my best attempt at a patch is the following:

diff --git a/lib/libcurses/newwin.c b/lib/libcurses/newwin.c
index 4db46cd..31cee08 100644
--- a/lib/libcurses/newwin.c
+++ b/lib/libcurses/newwin.c
@@ -135,9 +135,7 @@ __newwin(SCREEN *screen, int nlines, int ncols, int by, int bx, int ispad,
 
 	win->bch = ' ';
 	if (__using_color)
-		win->battr = __default_color;
-	else
-		win->battr = 0;
+		win->battr |= __default_color;
 	win->nextp = win;
 	win->ch_off = 0;
 	win->orig = NULL;
@@ -377,6 +375,7 @@ __makenew(SCREEN *screen, int nlines, int ncols, int by, int bx, int sub,
 	win->flags = (__IDLINE | __IDCHAR);
 	win->delay = -1;
 	win->wattr = 0;
+	win->battr = 0;
 #ifdef HAVE_WCHAR
 	win->bnsp = NULL;
 	SET_BGWCOL(*win, 1);