gdb: introduce rgb_color type to simplify existing code

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.

Approved-By: Tom Tromey <tom@tromey.com>

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);
 }
 

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,

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;
 

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)

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.  */
@@ -226,11 +223,7 @@ ui_file_style::color::approximate (const std::vector<color_space> &spaces) const
       target_space = sp;
 
   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;
   switch (target_space)
@@ -251,14 +244,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]);

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

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 ());