[PATCH v2 2/9] gdb: introduce rgb_color type to simplify existing code

Matthieu Longo
2026-03-03T16:20:05Z

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/python/py-color.c | 10 ++++-----
gdb/tui/tui-io.c | 3 +--
gdb/ui-style.c | 33 +++++++++++-----------------
gdb/ui-style.h | 38 ++++++++++++++++++++++++++++++++-
gdb/unittests/style-selftests.c | 10 ++++-----
5 files changed, 60 insertions(+), 34 deletions(-)

diff --git a/gdb/python/py-color.c b/gdb/python/py-color.c
index 24589dba265..69c7a2b5860 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..0952b89f4b7 100644
--- a/gdb/ui-style.c
+++ b/gdb/ui-style.c
@@ -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));
+ memcpy (rgb, palette_8colors[m_value], rgb.size_bytes ());
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));
+ memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());
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));
+ memcpy (rgb, palette_16colors[m_value], rgb.size_bytes ());
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..72349256034 100644
--- a/gdb/ui-style.h
+++ b/gdb/ui-style.h
@@ -54,6 +54,34 @@ 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 ();

+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 size_t size_bytes () const noexcept
+ { return m_data.size () * sizeof (decltype (m_data)::value_type); }
+
+ 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 +159,14 @@ struct ui_file_style
c, range.first, range.second, static_cast<int> (cs));
}

+ 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 +252,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