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>

diff --git a/gdb/guile/scm-color.c b/gdb/guile/scm-color.c
index c3a1d6632157132f4dd73c82ca1291d1e285782e..4ffe303578cf8881c595fee18684d01ee6aa02a1 100644 (file)
--- 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 efad50c4cbeb4d8bb955d474fb9841ecb2a957de..90ebe252f57ec1a2b9cf3d3833b1f160509635f9 100644 (file)
--- 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 04f7addec2f67631775a678b6ba7e6acdead1659..971209958cf7da0eff5c4c4f8564f9a043bc06b6 100644 (file)
--- 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 f673fbf36f6d2b3db240de327c66368e64dfc435..a9a50446e8a779c55cfe7ab89e6f898b7cceb6db 100644 (file)
--- 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 7ab466e24073f23090a3f6a57650e241829d2e6f..a191c753b08806ea58a8d18f9ed08dcaff2f3865 100644 (file)
--- 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.  */
@@ -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]);

diff --git a/gdb/ui-style.h b/gdb/ui-style.h
index fca9150889bd4924d4405ce83ed057a93f87dcc8..a1d656cd39cfd4e59c23535ba2f6e7ddbaa2ba22 100644 (file)
--- 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 f10a24d421746cc623aac10a6aaab507aed53a55..9035050bd6ce0a6fc718f942bf94d7c1a5680017 100644 (file)
--- 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 ());