Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Matthieu Longo <matthieu.longo@arm.com>
To: <gdb-patches@sourceware.org>, Tom Tromey <tom@tromey.com>
Cc: Matthieu Longo <matthieu.longo@arm.com>
Subject: [PATCH v3 1/7] gdb: introduce rgb_color type to simplify existing code
Date: Mon, 9 Mar 2026 17:56:18 +0000	[thread overview]
Message-ID: <20260309175624.236491-2-matthieu.longo@arm.com> (raw)
In-Reply-To: <20260309175624.236491-1-matthieu.longo@arm.com>

This patch replaces the raw uint8[3] buffer used to represent RGB values
with a more convenient wrapper, rgb_color, around std::array<uint8_t, 3>.
It also changes the return type of ui_file_style::color::get_rgb to
rgb_color instead of filling a caller-provided buffer, and updates all
callers accordingly.

This expected benefit of this change consists in:
- removing the manual size handling.
- proving accessors without using hard-coded indexes.
- making the API safer.
- simplifying call sites.

This refactoring does not introduce any functional change.
---
 gdb/guile/scm-color.c           |  9 ++++----
 gdb/mingw-hdep.c                |  5 ++---
 gdb/python/py-color.c           | 10 ++++-----
 gdb/tui/tui-io.c                |  3 +--
 gdb/ui-style.c                  | 37 +++++++++++++--------------------
 gdb/ui-style.h                  | 37 ++++++++++++++++++++++++++++++++-
 gdb/unittests/style-selftests.c | 10 ++++-----
 7 files changed, 67 insertions(+), 44 deletions(-)

diff --git a/gdb/guile/scm-color.c b/gdb/guile/scm-color.c
index c3a1d663215..4ffe303578c 100644
--- a/gdb/guile/scm-color.c
+++ b/gdb/guile/scm-color.c
@@ -339,11 +339,10 @@ gdbscm_color_components (SCM self)
   if (!color.is_direct ())
     gdbscm_misc_error (FUNC_NAME, SCM_ARG1, self, "color is not direct");
 
-  uint8_t rgb[3] = {};
-  color.get_rgb (rgb);
-  SCM red = scm_from_uint8 (rgb[0]);
-  SCM green = scm_from_uint8 (rgb[1]);
-  SCM blue = scm_from_uint8 (rgb[2]);
+  rgb_color rgb = color.get_rgb ();
+  SCM red = scm_from_uint8 (rgb.r ());
+  SCM green = scm_from_uint8 (rgb.g ());
+  SCM blue = scm_from_uint8 (rgb.b ());
   return scm_list_3 (red, green, blue);
 }
 
diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c
index efad50c4cbe..90ebe252f57 100644
--- a/gdb/mingw-hdep.c
+++ b/gdb/mingw-hdep.c
@@ -192,11 +192,10 @@ gdb_select (int n, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
 static int
 rgb_to_16colors (const ui_file_style::color &color)
 {
-  uint8_t rgb[3];
-  color.get_rgb (rgb);
+  rgb_color rgb = color.get_rgb ();
 
   int retval = 0;
-  for (int i = 0; i < 3; i++)
+  for (int i = 0; i < rgb.size (); i++)
     {
       /* Subdivide 256 possible values of each RGB component into 3
 	 regions: no color, normal color, bright color.  256 / 3 = 85,
diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 04f7addec2f..971209958cf 100644
--- a/gdb/python/py-color.c
+++ b/gdb/python/py-color.c
@@ -108,11 +108,9 @@ get_attr (PyObject *obj, PyObject *attr_name)
   if (color.is_direct ()
       && !PyUnicode_CompareWithASCIIString (attr_name, "components"))
     {
-      uint8_t rgb[3];
-      color.get_rgb (rgb);
-
-      gdbpy_ref<> rgb_objects[3];
-      for (int i = 0; i < 3; ++i)
+      rgb_color rgb = color.get_rgb ();
+      std::array<gdbpy_ref<>, rgb.size ()> rgb_objects;
+      for (auto i = 0u; i < rgb_objects.size (); ++i)
 	{
 	  rgb_objects[i] = gdb_py_object_from_ulongest (rgb[i]);
 	  if (rgb_objects[i] == nullptr)
@@ -123,7 +121,7 @@ get_attr (PyObject *obj, PyObject *attr_name)
       if (comp == nullptr)
 	return nullptr;
 
-      for (int i = 0; i < 3; ++i)
+      for (auto i = 0u; i < rgb_objects.size (); ++i)
 	if (PyTuple_SetItem (comp.get (), i, rgb_objects[i].release ()) < 0)
 	  return nullptr;
 
diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index f673fbf36f6..a9a50446e8a 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -255,8 +255,7 @@ get_color (const ui_file_style::color &color, int *result)
 	  int next = color_map.size () + 8;
 	  if (next >= COLORS)
 	    return false;
-	  uint8_t rgb[3];
-	  color.get_rgb (rgb);
+	  rgb_color rgb = color.get_rgb ();
 	  /* We store RGB as 0..255, but curses wants 0..1000.  */
 	  if (init_color (next, rgb[0] * 1000 / 255, rgb[1] * 1000 / 255,
 			  rgb[2] * 1000 / 255) == ERR)
diff --git a/gdb/ui-style.c b/gdb/ui-style.c
index 7ab466e2407..05725e314e4 100644
--- a/gdb/ui-style.c
+++ b/gdb/ui-style.c
@@ -53,7 +53,7 @@ static compiled_regex ansi_regex (ansi_regex_text, REG_EXTENDED,
 /* This maps 8-color palette to RGB triples.  The values come from
    plain linux terminal.  */
 
-static const uint8_t palette_8colors[][3] = {
+static const rgb_color palette_8colors[] = {
   { 1, 1, 1 },			/* Black.  */
   { 222, 56, 43 },		/* Red.  */
   { 57, 181, 74 },		/* Green.  */
@@ -66,7 +66,7 @@ static const uint8_t palette_8colors[][3] = {
 
 /* This maps 16-color palette to RGB triples.  The values come from xterm.  */
 
-static const uint8_t palette_16colors[][3] = {
+static const rgb_color palette_16colors[] = {
   { 0, 0, 0 },			/* Black.  */
   { 205, 0, 0 },		/* Red.  */
   { 0, 205, 0 },		/* Green.  */
@@ -166,25 +166,22 @@ ui_file_style::color::to_string () const
 
 /* See ui-style.h.  */
 
-void
-ui_file_style::color::get_rgb (uint8_t *rgb) const
+rgb_color
+ui_file_style::color::get_rgb () const
 {
+  rgb_color rgb;
   if (m_color_space == color_space::RGB_24BIT)
-    {
-      rgb[0] = m_red;
-      rgb[1] = m_green;
-      rgb[2] = m_blue;
-    }
+    rgb = rgb_color (m_red, m_green, m_blue);
   else if (m_color_space == color_space::ANSI_8COLOR
 	   && 0 <= m_value && m_value <= 7)
-    memcpy (rgb, palette_8colors[m_value], 3 * sizeof (uint8_t));
+    rgb = palette_8colors[m_value];
   else if (m_color_space == color_space::AIXTERM_16COLOR
 	   && 0 <= m_value && m_value <= 15)
-    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
+    rgb = palette_16colors[m_value];
   else if (m_color_space != color_space::XTERM_256COLOR)
     gdb_assert_not_reached ("get_rgb called on invalid color");
   else if (0 <= m_value && m_value <= 15)
-    memcpy (rgb, palette_16colors[m_value], 3 * sizeof (uint8_t));
+    rgb = palette_16colors[m_value];
   else if (m_value >= 16 && m_value <= 231)
     {
       int value = m_value;
@@ -202,12 +199,12 @@ ui_file_style::color::get_rgb (uint8_t *rgb) const
   else if (232 <= m_value && m_value <= 255)
     {
       uint8_t v = (m_value - 232) * 10 + 8;
-      rgb[0] = v;
-      rgb[1] = v;
-      rgb[2] = v;
+      rgb = rgb_color (v, v, v);
     }
   else
     gdb_assert_not_reached ("get_rgb called on invalid color");
+
+  return rgb;
 }
 
 /* See ui-style.h.  */
@@ -227,9 +224,7 @@ ui_file_style::color::approximate (const std::vector<color_space> &spaces) const
 
   if (target_space == color_space::RGB_24BIT)
     {
-      uint8_t rgb[3];
-      get_rgb (rgb);
-      return color (rgb[0], rgb[1], rgb[2]);
+      return color (get_rgb ());
     }
 
   int target_size = 0;
@@ -251,14 +246,12 @@ ui_file_style::color::approximate (const std::vector<color_space> &spaces) const
 
   color result = NONE;
   int best_distance = std::numeric_limits<int>::max ();
-  uint8_t rgb[3];
-  get_rgb (rgb);
+  rgb_color rgb = get_rgb ();
 
   for (int i = 0; i < target_size; ++i)
     {
-      uint8_t c_rgb[3];
       color c (target_space, i);
-      c.get_rgb (c_rgb);
+      rgb_color c_rgb = c.get_rgb ();
       int d_red = std::abs (rgb[0] - c_rgb[0]);
       int d_green = std::abs (rgb[1] - c_rgb[1]);
       int d_blue = std::abs (rgb[2] - c_rgb[2]);
diff --git a/gdb/ui-style.h b/gdb/ui-style.h
index fca9150889b..a1d656cd39c 100644
--- a/gdb/ui-style.h
+++ b/gdb/ui-style.h
@@ -54,6 +54,33 @@ extern bool color_space_safe_cast (color_space *result, long c);
 /* Get the number of colors supported by the terminal where GDB is running.  */
 extern int gdb_get_ncolors ();
 
+/* Convenient wrapper for RGB color values.  */
+struct rgb_color
+{
+private:
+  std::array <uint8_t, 3> m_data;
+
+public:
+  constexpr rgb_color ()
+    : m_data {}
+  {}
+  constexpr rgb_color (uint8_t r, uint8_t g, uint8_t b)
+    : m_data {r, g, b}
+  {}
+
+  constexpr uint8_t r () const noexcept { return m_data[0]; }
+  constexpr uint8_t g () const noexcept { return m_data[1]; }
+  constexpr uint8_t b () const noexcept { return m_data[2]; }
+
+  constexpr operator uint8_t *() noexcept { return m_data.data (); }
+  constexpr size_t size () const noexcept { return m_data.size (); }
+
+  constexpr uint8_t &operator[] (std::size_t idx) noexcept
+  { return m_data[idx]; }
+  constexpr const uint8_t &operator[] (std::size_t idx) const noexcept
+  { return m_data[idx]; }
+};
+
 /* Styles that can be applied to a ui_file.  */
 struct ui_file_style
 {
@@ -131,6 +158,14 @@ struct ui_file_style
 	       c, range.first, range.second, static_cast<int> (cs));
     }
 
+    explicit color (const rgb_color &rgb)
+      : m_color_space (color_space::RGB_24BIT),
+	m_red (rgb.r ()),
+	m_green (rgb.g ()),
+	m_blue (rgb.b ())
+    {
+    }
+
     color (uint8_t r, uint8_t g, uint8_t b)
       : m_color_space (color_space::RGB_24BIT),
 	m_red (r),
@@ -216,7 +251,7 @@ struct ui_file_style
     /* Fill in RGB with the red/green/blue values for this color.
        This may not be called for basic colors or for the "NONE"
        color.  */
-    void get_rgb (uint8_t *rgb) const;
+    rgb_color get_rgb () const;
 
     /* Append the ANSI terminal escape sequence for this color to STR.
        IS_FG indicates whether this is a foreground or background
diff --git a/gdb/unittests/style-selftests.c b/gdb/unittests/style-selftests.c
index f10a24d4217..9035050bd6c 100644
--- a/gdb/unittests/style-selftests.c
+++ b/gdb/unittests/style-selftests.c
@@ -31,7 +31,7 @@ run_tests ()
 {
   ui_file_style style;
   size_t n_read;
-  uint8_t rgb[3];
+  rgb_color rgb;
 
   SELF_CHECK (style.parse ("\033[m", &n_read));
   SELF_CHECK (n_read == 3);
@@ -94,10 +94,10 @@ run_tests ()
   SELF_CHECK (style.parse ("\033[38;5;112;48;5;249m", &n_read));
   SELF_CHECK (n_read == 20);
   SELF_CHECK (!style.get_foreground ().is_basic ());
-  style.get_foreground ().get_rgb (rgb);
+  rgb = style.get_foreground ().get_rgb ();
   CHECK_RGB (0x87, 0xd7, 0);
   SELF_CHECK (!style.get_background ().is_basic ());
-  style.get_background ().get_rgb (rgb);
+  rgb = style.get_background ().get_rgb ();
   CHECK_RGB (0xb2, 0xb2, 0xb2);
   SELF_CHECK (style.get_intensity () == ui_file_style::NORMAL);
   SELF_CHECK (!style.is_italic ());
@@ -109,10 +109,10 @@ run_tests ()
   SELF_CHECK (style.parse ("\033[38;2;83;84;85;48;2;0;1;254;2;7m", &n_read));
   SELF_CHECK (n_read == 33);
   SELF_CHECK (!style.get_foreground ().is_basic ());
-  style.get_foreground ().get_rgb (rgb);
+  rgb = style.get_foreground ().get_rgb ();
   CHECK_RGB (83, 84, 85);
   SELF_CHECK (!style.get_background ().is_basic ());
-  style.get_background ().get_rgb (rgb);
+  rgb = style.get_background ().get_rgb ();
   CHECK_RGB (0, 1, 254);
   SELF_CHECK (style.get_intensity () == ui_file_style::DIM);
   SELF_CHECK (!style.is_italic ());
-- 
2.53.0


  reply	other threads:[~2026-03-09 17:58 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 17:56 [PATCH v3 0/7] gdb: more fixes for Python limited C API support Matthieu Longo
2026-03-09 17:56 ` Matthieu Longo [this message]
2026-03-09 19:22   ` [PATCH v3 1/7] gdb: introduce rgb_color type to simplify existing code Tom Tromey
2026-03-11 15:03     ` Simon Marchi
2026-03-11 17:55       ` Matthieu Longo
2026-03-11 18:04         ` Simon Marchi
2026-03-11 18:16         ` Tom Tromey
2026-03-09 17:56 ` [PATCH v3 2/7] gdb: fail configure if Python version is too old for limited API Matthieu Longo
2026-03-24 18:53   ` Tom Tromey
2026-03-31 10:16     ` Matthieu Longo
2026-04-07 13:36       ` Matthieu Longo
2026-04-07 20:39       ` Tom Tromey
2026-03-09 17:56 ` [PATCH v3 3/7] gdb: add new helpers for retrieving a type's fully qualified name Matthieu Longo
2026-03-24 18:44   ` Tom Tromey
2026-03-09 17:56 ` [PATCH v3 4/7] gdb/python: allow ref_ptr<T, Policy>::new_reference to accept subclasses of T Matthieu Longo
2026-03-09 19:40   ` Tom Tromey
2026-03-10 12:26     ` Matthieu Longo
2026-03-09 17:56 ` [PATCH v3 5/7] gdb/python: flatten functions calling PyObject_New and use gdbpy_ref Matthieu Longo
2026-03-09 20:07   ` Tom Tromey
2026-03-09 17:56 ` [PATCH v3 6/7] gdb/python: add gdbpy_dict_wrapper:allocate_dict helper Matthieu Longo
2026-03-09 20:09   ` Tom Tromey
2026-03-10 12:26     ` Matthieu Longo
2026-03-09 17:56 ` [PATCH v3 7/7] gdb/python: add accessor helpers for __dict__ in Python extension objects Matthieu Longo
2026-03-13 18:09   ` Tom Tromey
2026-03-23 10:28 ` [PATCH v3 0/7] gdb: more fixes for Python limited C API support Matthieu Longo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260309175624.236491-2-matthieu.longo@arm.com \
    --to=matthieu.longo@arm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=tom@tromey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox