Re: [PATCH] gdb/tui: Fix unexpected reuse of color pairs

["Jakob Schäffeler" <jakob.schaeffeler@tum.de>]

[-- Attachment #1: Type: text/plain, Size: 5052 bytes --]

I addressed all your comments in a new version of the patch in
https://sourceware.org/pipermail/gdb-patches/2026-May/227275.html 

However, I needed to figure out who at my company could sign the
copyright assignment so this took a while. But this is now done.

Do you need anything else from my side?

Thanks,
Jakob 


On Fri, 2026-05-08 at 10:23 +0100, Andrew Burgess wrote:
> 
> Thanks for working on this.  This looks like it could be a good
> improvement.
> 
> jakob.schaeffeler@tum.de writes:
> 
> > From: Jakob Schäffeler <jakob.schaeffeler@tum.de>
> 
> Commit message text should be line wrapped at around 72 characters,
> your
> commit message will need to be reformatted so that it is readable in
> 'git log' output.
> 
> > 
> > TUI translates ANSI styling sequences to curses color pairs.
> > Currently in this process uses COLOR_PAIR which only returns values
> > from 0 to 255 which results in unexpected reuse of color pairs.
> 
> typo: "Currently, this process uses COLOR_PAIR, which only returns
> values ..."
> 
> > This patch avoids calling COLOR_PAIR(pair) to be able to render
> > more than 256 color pairs. For this the wattron call is replaced
> > with wcolor_set.
> > This also results in last_color_pair no longer being needed since
> > we set the color directly with wcolor_set and do not need
> > wattron/off pairs any longer.
> 
> > This results in SHRT_MAX different color pairs to be available. To
> > get
> > all 65535 color pairs init_pair is replaced with
> > init_extended_pair,
> > which returns an int instead of short.
> 
> typo: "...which TAKES int instead of short.".  They both return
> 'int'.
> 
> > This patch was tested with make check-gdb TESTS="gdb.tui/*.exp"
> 
> It would be good if there was a new test added which checks the new
> extended colour range.  Is there a reason why this cannot be done?
> 
> I also wonder how widely available this extended colour API is?  Is
> this
> supported on mingw?  Or FreeBSD?  Or Solaris (is this even
> used/supported these days)?  I wonder if we should be adding
> configure
> checks for this API, or if it's OK to just put a hard requirement in
> place?   At the very least it would be nice to document in the commit
> message if nowhere else, what version/package requirements this is
> now
> placing on us.
> 
> Also, I suspect this patch is probably more than trivial, so we will
> probably need a copyright assignment in agreement before we could
> accept
> it.  Information on this process can be found here:
> 
> https://sourceware.org/gdb/wiki/ContributionChecklist#FSF_copyright_Assignment
> 
> If you have any questions, feel free to ask and we can help out.
> 
> Thanks,
> Andrew
> 
> 
> > 
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=34134
> > ---
> >  gdb/tui/tui-io.c | 16 +++-------------
> >  1 file changed, 3 insertions(+), 13 deletions(-)
> > 
> > diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> > index 642b88ead0c..5e239179b5e 100644
> > --- a/gdb/tui/tui-io.c
> > +++ b/gdb/tui/tui-io.c
> > @@ -265,10 +265,6 @@ get_color (const ui_file_style::color &color,
> > int *result)
> >    return true;
> >  }
> >  
> > -/* The most recently emitted color pair.  */
> > -
> > -static int last_color_pair = -1;
> > -
> >  /* The most recently applied style.  */
> >  
> >  static ui_file_style last_style;
> > @@ -299,7 +295,7 @@ get_color_pair (int fg, int bg)
> >  	 back to the default if we've used too many.  */
> >        if (next >= COLOR_PAIRS)
> >  	return 0;
> > -      init_pair (next, fg, bg);
> > +      init_extended_pair (next, fg, bg);
> >        color_pair_map[c] = next;
> >        return next;
> >      }
> > @@ -320,9 +316,7 @@ tui_apply_style (WINDOW *w, ui_file_style
> > style)
> >  #endif
> >    wattroff (w, A_UNDERLINE);
> >    wattroff (w, A_REVERSE);
> > -  if (last_color_pair != -1)
> > -    wattroff (w, COLOR_PAIR (last_color_pair));
> > -  wattron (w, COLOR_PAIR (0));
> > +  wcolor_set (w, 0, nullptr);
> >  
> >    const ui_file_style::color &fg = style.get_foreground ();
> >    const ui_file_style::color &bg = style.get_background ();
> > @@ -342,10 +336,7 @@ tui_apply_style (WINDOW *w, ui_file_style
> > style)
> >  	    bgi = (ncurses_norm_attr >> 4) & 15;
> >  #endif
> >  	  int pair = get_color_pair (fgi, bgi);
> > -	  if (last_color_pair != -1)
> > -	    wattroff (w, COLOR_PAIR (last_color_pair));
> > -	  wattron (w, COLOR_PAIR (pair));
> > -	  last_color_pair = pair;
> > +	  wcolor_set (w, 0, &pair);
> >  	}
> >      }
> >  
> > @@ -907,7 +898,6 @@ tui_setup_io (int mode)
> >        savetty ();
> >  
> >        /* Clean up color information.  */
> > -      last_color_pair = -1;
> >        last_style = ui_file_style ();
> >        color_map.clear ();
> >        color_pair_map.clear ();
> > -- 
> > 2.54.0

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3517 bytes --]