Re: [PATCH 3/9] gmp-utils: New API to simply use of GMP's integer/rational/float objects

[Joel Brobecker <brobecker@adacore.com>]

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

Hi Simon,

[copying Tom as well, as he helped me a lot with the C++ part
when designing this API]

> Here are some comments.  Most of them are really just suggestions,
> what you have looks good to me.  The suggestions can easily be
> implemented later, so you don't have to re-do your patch series.

Thanks for your thorough review.

I took a bit of time today to look at your comments, and work on them.
I'll fold the trivial changes in v2 of the patch. For the more impactful
changes, I'll make them separate patches at the end of the patch series.
Hopefully that'll simplify the review process both for the existing
patch as well as the proposed changes.

> > +/* Same as gmp_asprintf, but returning a convenient wrapper type.  */
> > +
> > +gdb::unique_xmalloc_ptr<char> gmp_string_asprintf (const char *fmt, ...);
> 
> I don't know how gmp_string_asprintf will be used, but would it make
> sense to make it return an std::string?  Does gmp_sprintf support
> passing NULL as BUF, so that you could replicate what is done in
> string_printf?

I can't remember the exact details behind the choice of using
a unique_xmalloc_ptr, but I think it was to avoid having an extra
malloc/free when going from the memory allocated by gmp_vasprintf
to the data returned to caller.

I don't think this function is expected to be called that often,
so it sounds worth the trouble making the API more C++-y.
I've attached a copy of the patch that does that as "0001-[...]".

> > +/* A class to make it easier to use GMP's mpz_t values within GDB.  */
> > +
> > +struct gdb_mpz
> > +{
> > +  mpz_t val;
> 
> I don't know what's coming in the following patches, but would it work
> to make the field private and only use it through methods?  Having it
> totally encapsulated would make me more confident that the callers don't
> do anything they are not supposed to with it.
> 
> There would need to be methods for arithmetic operations that we use
> (e.g. mpz_add), but we don't have to add methods for all existing
> functions in gmp, just the ones we use.  And I presume we don't use that
> many.

I'm not sure about that. The main purpose of these classes is to automate
the lifetime of the data allocated when creating the underlying GMP objects.
Beyond that, there are a few help routines that are connected as methods
because it makes sense to do so now that we have the classes, but could
have otherwise been just regular functions.

I am not opposed to the idea of making the GMP objects private,
but I don't really see enough benefits...

> > +  gdb_mpz &operator== (gdb_mpz &&other)
> > +  {
> > +    mpz_swap (val, other.val);
> > +    return *this;
> > +  }
> 
> Is this meant to be "operator="?

Nice catch! I think so, and I'll fix if Tom confirms as well.

> > +  /* Set VAL by importing the number stored in the byte buffer (BUF),
> > +     given its size (LEN) and BYTE_ORDER.
> > +
> > +     UNSIGNED_P indicates whether the number has an unsigned type.  */
> > +  void read (const gdb_byte *buf, int len, enum bfd_endian byte_order,
> > +	     bool unsigned_p);
> > +
> > +  /* Write VAL into BUF as a LEN-bytes number with the given BYTE_ORDER.
> > +
> > +     UNSIGNED_P indicates whether the number has an unsigned type.  */
> > +  void write (gdb_byte *buf, int len, enum bfd_endian byte_order,
> > +	      bool unsigned_p) const;
> 
> These two would be good candidates for gdb::array_view.

If we change these, I think it would make sense to change the
read_fixed_point/write_fixed_point duo as well.

I'm on the fence about this suggestion. Attached is the patch
implementing it. On the plus side, the API is more idiomatic-C++.
One the down side, it very so slightly increases the complexity
of both the implementation and the current callers. It's because
the rest of the infrastructure is still structured in a way that
what we have access to are pointers and sizes. Perhaps one could
say that, even though the point of call is currently a bit more
work, it does help convey the idea that the "len" is the len of
the buffer.

I don't have a strong opinion either way, so I'm happy to follow
what people think. The patch will be included in v2 of the patch
series, and I'm happy to keep or drop.

-- 
Joel

[-- Attachment #2: 0001-change-gmp_string_asprintf-to-return-an-std-string.patch --]
[-- Type: text/x-diff, Size: 4582 bytes --]

From 6f2bf2c0dc375788b739921f74e1b6da73bf78ec Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Fri, 13 Nov 2020 02:49:06 -0500
Subject: [PATCH 1/2] change gmp_string_asprintf to return an std::string

This was suggested by Simon during a code review of this package upstream.
The upside is that this makes the function's API more natural and C++.
The downside is an extra malloc, which might be the reason why we went
for using a unique_xmalloc_ptr in the first place. Since this function
is not expected to be called frequently, the API improvement might be
worth the performance impact.

gdb/ChangeLog:

        * gmp-utils.h (gmp_string_asprintf): Change return type to
        std::string. Update all callers.
        * gmp-utils.c (gmp_string_asprintf): Likewise.

Change-Id: I49c21053c595583d2110ba2a730e3eeb8e38155d
TN: NB04-020
---
 gdb/gdbtypes.c  |  2 +-
 gdb/gmp-utils.c |  6 ++++--
 gdb/gmp-utils.h | 10 ++++------
 gdb/typeprint.c |  5 ++---
 gdb/valprint.c  |  4 ++--
 5 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index a3a6f07..58c4bc4 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -4922,7 +4922,7 @@ static void
 print_fixed_point_type_info (struct type *type, int spaces)
 {
   printfi_filtered (spaces + 2, "scaling factor: %s\n",
-		    fixed_point_scaling_factor (type).str ().get ());
+		    fixed_point_scaling_factor (type).str ().c_str ());
 }
 
 static struct obstack dont_print_type_obstack;
diff --git a/gdb/gmp-utils.c b/gdb/gmp-utils.c
index db92e57..b70aaa3 100644
--- a/gdb/gmp-utils.c
+++ b/gdb/gmp-utils.c
@@ -19,7 +19,7 @@
 
 /* See gmp-utils.h.  */
 
-gdb::unique_xmalloc_ptr<char>
+std::string
 gmp_string_asprintf (const char *fmt, ...)
 {
   va_list vp;
@@ -29,7 +29,9 @@ gmp_string_asprintf (const char *fmt, ...)
   gmp_vasprintf (&buf, fmt, vp);
   va_end (vp);
 
-  return gdb::unique_xmalloc_ptr<char> (buf);
+  std::string result(buf);
+  xfree (buf);
+  return result;
 }
 
 /* See gmp-utils.h.  */
diff --git a/gdb/gmp-utils.h b/gdb/gmp-utils.h
index 1214b64..7eed29a 100644
--- a/gdb/gmp-utils.h
+++ b/gdb/gmp-utils.h
@@ -29,9 +29,9 @@
 #include <gmp.h>
 #include "gdbsupport/traits.h"
 
-/* Same as gmp_asprintf, but returning a convenient wrapper type.  */
+/* Same as gmp_asprintf, but returning an std::string.  */
 
-gdb::unique_xmalloc_ptr<char> gmp_string_asprintf (const char *fmt, ...);
+std::string gmp_string_asprintf (const char *fmt, ...);
 
 /* A class to make it easier to use GMP's mpz_t values within GDB.  */
 
@@ -110,8 +110,7 @@ struct gdb_mpz
 	      bool unsigned_p) const;
 
   /* Return a string containing VAL.  */
-  gdb::unique_xmalloc_ptr<char> str () const
-  { return gmp_string_asprintf ("%Zd", val); }
+  std::string str () const { return gmp_string_asprintf ("%Zd", val); }
 
   /* The destructor.  */
   ~gdb_mpz () { mpz_clear (val); }
@@ -163,8 +162,7 @@ struct gdb_mpq
   }
 
   /* Return a string representing VAL as "<numerator> / <denominator>".  */
-  gdb::unique_xmalloc_ptr<char> str () const
-  { return gmp_string_asprintf ("%Qd", val); }
+  std::string str () const { return gmp_string_asprintf ("%Qd", val); }
 
   /* Return VAL rounded to the nearest integer.  */
   gdb_mpz get_rounded () const;
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index 6a07b33..6eedb5d 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -668,11 +668,10 @@ print_type_scalar (struct type *type, LONGEST val, struct ui_file *stream)
 void
 print_type_fixed_point (struct type *type, struct ui_file *stream)
 {
-  gdb::unique_xmalloc_ptr<char> small_img
-    = fixed_point_scaling_factor (type).str ();
+  std::string small_img = fixed_point_scaling_factor (type).str ();
 
   fprintf_filtered (stream, "%s-byte fixed point (small = %s)",
-		    pulongest (TYPE_LENGTH (type)), small_img.get ());
+		    pulongest (TYPE_LENGTH (type)), small_img.c_str ());
 }
 
 /* Dump details of a type specified either directly or indirectly.
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 38ae0bd..abef002 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -814,8 +814,8 @@ generic_val_print_fixed_point (struct value *val, struct ui_file *stream,
 			  fixed_point_scaling_factor (type));
 
       const char *fmt = TYPE_LENGTH (type) < 4 ? "%.11Fg" : "%.17Fg";
-      gdb::unique_xmalloc_ptr<char> str = gmp_string_asprintf (fmt, f.val);
-      fprintf_filtered (stream, "%s", str.get ());
+      std::string str = gmp_string_asprintf (fmt, f.val);
+      fprintf_filtered (stream, "%s", str.c_str ());
     }
 }
 
-- 
2.1.4


[-- Attachment #3: 0002-gmp-utils-Convert-the-read-write-methods-to-using-gd.patch --]
[-- Type: text/x-diff, Size: 14455 bytes --]

From 3260f64e727e699ce98a02a0a3fe9859c41ab059 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Fri, 13 Nov 2020 02:17:15 -0500
Subject: [PATCH 2/2] gmp-utils: Convert the read/write methods to using
 gdb::array_view

This commit changes the interfaces of some of the methods declared
in gmp-utils to take a gdb::array_view of gdb_byte instead of a
(gdb_byte *, size) couple.

This makes these methods' API probably more C++-idiomatic.
With the way things are structured, this change introduces a minor
extra complication at the point of call of these methods, since
the data available there is not in the form of an array_view,
and thus the array_view needs to be constructed on the spot.

        * gmp-utils.h (gdb_mpz::read): Change buf and len parameters
        into one single gdb::array_view parameter.
        (gdb_mpz::write): Likewise.
        (gdb_mpq::read_fixed_point, gdb_mpq::write_fixed_point): Likewise.
        * gmp-utils.c (gdb_mpz::read): Change buf and len parameters
        into one single gdb::array_view parameter.
        Adjust implementation accordingly.
        (gdb_mpz::write): Likewise.
        (gdb_mpq::read_fixed_point, gdb_mpq::write_fixed_point): Likewise.
        * unittests/gmp-utils-selftests.c: Adapt following changes above.
        * valarith.c, valops.c, valprint.c, value.c: Likewise.

Change-Id: Ia6e9f077def06b92e089684164066fc81fff5e29
---
 gdb/gmp-utils.c                     | 25 +++++++++++++------------
 gdb/gmp-utils.h                     | 32 ++++++++++++++++++--------------
 gdb/unittests/gmp-utils-selftests.c | 12 ++++++++----
 gdb/valarith.c                      |  9 ++++++---
 gdb/valops.c                        | 11 +++++++----
 gdb/valprint.c                      |  3 ++-
 gdb/value.c                         |  3 ++-
 7 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/gdb/gmp-utils.c b/gdb/gmp-utils.c
index b70aaa3..6d80f13 100644
--- a/gdb/gmp-utils.c
+++ b/gdb/gmp-utils.c
@@ -37,12 +37,12 @@ gmp_string_asprintf (const char *fmt, ...)
 /* See gmp-utils.h.  */
 
 void
-gdb_mpz::read (const gdb_byte *buf, int len, enum bfd_endian byte_order,
+gdb_mpz::read (gdb::array_view<const gdb_byte> buf, enum bfd_endian byte_order,
 	       bool unsigned_p)
 {
-  mpz_import (val, 1 /* count */, -1 /* order */, len /* size */,
+  mpz_import (val, 1 /* count */, -1 /* order */, buf.size () /* size */,
 	      byte_order == BFD_ENDIAN_BIG ? 1 : -1 /* endian */,
-	      0 /* nails */, buf /* op */);
+	      0 /* nails */, buf.data () /* op */);
 
   if (!unsigned_p)
     {
@@ -51,7 +51,7 @@ gdb_mpz::read (const gdb_byte *buf, int len, enum bfd_endian byte_order,
 	 was in fact negative, we need to adjust VAL accordingly.  */
       gdb_mpz max;
 
-      mpz_ui_pow_ui (max.val, 2, len * TARGET_CHAR_BIT - 1);
+      mpz_ui_pow_ui (max.val, 2, buf.size () * TARGET_CHAR_BIT - 1);
       if (mpz_cmp (val, max.val) >= 0)
 	mpz_submul_ui (val, max.val, 2);
     }
@@ -60,7 +60,7 @@ gdb_mpz::read (const gdb_byte *buf, int len, enum bfd_endian byte_order,
 /* See gmp-utils.h.  */
 
 void
-gdb_mpz::write (gdb_byte *buf, int len, enum bfd_endian byte_order,
+gdb_mpz::write (gdb::array_view<gdb_byte> buf, enum bfd_endian byte_order,
 		bool unsigned_p) const
 {
   gdb_mpz exported_val (val);
@@ -72,14 +72,15 @@ gdb_mpz::write (gdb_byte *buf, int len, enum bfd_endian byte_order,
 	 would be the same as our negative value.  */
       gdb_mpz neg_offset;
 
-      mpz_ui_pow_ui (neg_offset.val, 2, len * TARGET_CHAR_BIT);
+      mpz_ui_pow_ui (neg_offset.val, 2, buf.size () * TARGET_CHAR_BIT);
       mpz_add (exported_val.val, exported_val.val, neg_offset.val);
     }
 
   /* Start by clearing the buffer, as mpz_export only writes as many
      bytes as it needs (including none, if the value to export is zero.  */
-  memset (buf, 0, len);
-  mpz_export (buf, NULL /* count */, -1 /* order */, len /* size */,
+  memset (buf.data (), 0, buf.size ());
+  mpz_export (buf.data (), NULL /* count */, -1 /* order */,
+	      buf.size () /* size */,
 	      byte_order == BFD_ENDIAN_BIG ? 1 : -1 /* endian */,
 	      0 /* nails */, exported_val.val);
 }
@@ -120,12 +121,12 @@ gdb_mpq::get_rounded () const
 /* See gmp-utils.h.  */
 
 void
-gdb_mpq::read_fixed_point (const gdb_byte *buf, int len,
+gdb_mpq::read_fixed_point (gdb::array_view<const gdb_byte> buf,
 			   enum bfd_endian byte_order, bool unsigned_p,
 			   const gdb_mpq &scaling_factor)
 {
   gdb_mpz vz;
-  vz.read (buf, len, byte_order, unsigned_p);
+  vz.read (buf, byte_order, unsigned_p);
 
   mpq_set_z (val, vz.val);
   mpq_mul (val, val, scaling_factor.val);
@@ -134,7 +135,7 @@ gdb_mpq::read_fixed_point (const gdb_byte *buf, int len,
 /* See gmp-utils.h.  */
 
 void
-gdb_mpq::write_fixed_point (gdb_byte *buf, int len,
+gdb_mpq::write_fixed_point (gdb::array_view<gdb_byte> buf,
 			    enum bfd_endian byte_order, bool unsigned_p,
 			    const gdb_mpq &scaling_factor) const
 {
@@ -143,7 +144,7 @@ gdb_mpq::write_fixed_point (gdb_byte *buf, int len,
   mpq_div (unscaled.val, unscaled.val, scaling_factor.val);
 
   gdb_mpz unscaled_z = unscaled.get_rounded ();
-  unscaled_z.write (buf, len, byte_order, unsigned_p);
+  unscaled_z.write (buf, byte_order, unsigned_p);
 }
 
 /* A wrapper around xrealloc that we can then register with GMP
diff --git a/gdb/gmp-utils.h b/gdb/gmp-utils.h
index 7eed29a..ac6bb0d 100644
--- a/gdb/gmp-utils.h
+++ b/gdb/gmp-utils.h
@@ -96,17 +96,19 @@ struct gdb_mpz
      The return type can signed or unsigned, with no size restriction.  */
   template<typename T> T as_integer () const;
 
-  /* Set VAL by importing the number stored in the byte buffer (BUF),
-     given its size (LEN) and BYTE_ORDER.
+  /* Set VAL by importing the number stored in the byte array (BUF),
+     using the given BYTE_ORDER.  The size of the data to read is
+     the byte array's size.
 
      UNSIGNED_P indicates whether the number has an unsigned type.  */
-  void read (const gdb_byte *buf, int len, enum bfd_endian byte_order,
+  void read (gdb::array_view<const gdb_byte> buf, enum bfd_endian byte_order,
 	     bool unsigned_p);
 
-  /* Write VAL into BUF as a LEN-bytes number with the given BYTE_ORDER.
+  /* Write VAL into BUF as a number whose byte size is the size of BUF,
+     using the given BYTE_ORDER.
 
      UNSIGNED_P indicates whether the number has an unsigned type.  */
-  void write (gdb_byte *buf, int len, enum bfd_endian byte_order,
+  void write (gdb::array_view<gdb_byte> buf, enum bfd_endian byte_order,
 	      bool unsigned_p) const;
 
   /* Return a string containing VAL.  */
@@ -167,24 +169,26 @@ struct gdb_mpq
   /* Return VAL rounded to the nearest integer.  */
   gdb_mpz get_rounded () const;
 
-  /* Set VAL from the contents of the given buffer (BUF), which
-     contains the unscaled value of a fixed point type object
-     with the given size (LEN) and byte order (BYTE_ORDER).
+  /* Set VAL from the contents of the given byte array (BUF), which
+     contains the unscaled value of a fixed point type object.
+     The byte size of the data is the size of BUF.
+
+     BYTE_ORDER provides the byte_order to use when reading the data.
 
      UNSIGNED_P indicates whether the number has an unsigned type.
      SCALING_FACTOR is the scaling factor to apply after having
      read the unscaled value from our buffer.  */
-  void read_fixed_point (const gdb_byte *buf, int len,
+  void read_fixed_point (gdb::array_view<const gdb_byte> buf,
 			 enum bfd_endian byte_order, bool unsigned_p,
 			 const gdb_mpq &scaling_factor);
 
-  /* Write VAL into BUF as a LEN-bytes fixed point value following
-     the given BYTE_ORDER.
+  /* Write VAL into BUF as fixed point value following the given BYTE_ORDER.
+     The size of BUF is used as the length to write the value into.
 
      UNSIGNED_P indicates whether the number has an unsigned type.
      SCALING_FACTOR is the scaling factor to apply before writing
      the unscaled value to our buffer.  */
-  void write_fixed_point (gdb_byte *buf, int len,
+  void write_fixed_point (gdb::array_view<gdb_byte> buf,
 			  enum bfd_endian byte_order, bool unsigned_p,
 			  const gdb_mpq &scaling_factor) const;
 
@@ -213,13 +217,13 @@ struct gdb_mpf
      UNSIGNED_P indicates whether the number has an unsigned type.
      SCALING_FACTOR is the scaling factor to apply after having
      read the unscaled value from our buffer.  */
-  void read_fixed_point (const gdb_byte *buf, int len,
+  void read_fixed_point (gdb::array_view<const gdb_byte> buf,
 			 enum bfd_endian byte_order, bool unsigned_p,
 			 const gdb_mpq &scaling_factor)
   {
     gdb_mpq tmp_q;
 
-    tmp_q.read_fixed_point (buf, len, byte_order, unsigned_p, scaling_factor);
+    tmp_q.read_fixed_point (buf, byte_order, unsigned_p, scaling_factor);
     mpf_set_q (val, tmp_q.val);
   }
 
diff --git a/gdb/unittests/gmp-utils-selftests.c b/gdb/unittests/gmp-utils-selftests.c
index b5738eb..ccd7f01 100644
--- a/gdb/unittests/gmp-utils-selftests.c
+++ b/gdb/unittests/gmp-utils-selftests.c
@@ -109,7 +109,8 @@ store_and_read_back (T val, int buf_len, enum bfd_endian byte_order,
   mpz_set (actual.val, expected.val);
   mpz_sub_ui (actual.val, actual.val, 500);
 
-  actual.read (buf, buf_len, byte_order, !std::is_signed<T>::value);
+  actual.read (gdb::array_view<const gdb_byte> (buf, buf_len),
+	       byte_order, !std::is_signed<T>::value);
 }
 
 /* Test the gdb_mpz::read method over a reasonable range of values.
@@ -234,7 +235,8 @@ write_and_extract (T val, int buf_len, enum bfd_endian byte_order)
   SELF_CHECK (v.as_integer<T> () == val);
 
   gdb_byte *buf = (gdb_byte *) alloca (buf_len);
-  v.write (buf, buf_len, byte_order, !std::is_signed<T>::value);
+  v.write (gdb::array_view<gdb_byte> (buf, buf_len),
+	   byte_order, !std::is_signed<T>::value);
 
   return extract_integer<T> (buf, buf_len, byte_order);
 }
@@ -333,7 +335,8 @@ read_fp_test (int unscaled, const gdb_mpq &scaling_factor,
   gdb_byte buf[len];
   store_signed_integer (buf, len, byte_order, unscaled);
 
-  actual.read_fixed_point (buf, len, byte_order, 0, scaling_factor);
+  actual.read_fixed_point (gdb::array_view<const gdb_byte> (buf, len),
+			   byte_order, 0, scaling_factor);
 
   mpq_set_si (expected.val, unscaled, 1);
   mpq_mul (expected.val, expected.val, scaling_factor.val);
@@ -402,7 +405,8 @@ write_fp_test (int numerator, unsigned int denominator,
   gdb_mpq v;
   mpq_set_ui (v.val, numerator, denominator);
   mpq_canonicalize (v.val);
-  v.write_fixed_point (buf, len, byte_order, 0, scaling_factor);
+  v.write_fixed_point (gdb::array_view<gdb_byte> (buf, len),
+		       byte_order, 0, scaling_factor);
 
   return extract_unsigned_integer (buf, len, byte_order);
 }
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 0e38aed..7d406b1 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -939,10 +939,12 @@ fixed_point_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
     }
 
   gdb_mpq v1, v2, res;
-  v1.read_fixed_point (value_contents (arg1), TYPE_LENGTH (type1),
+  v1.read_fixed_point (gdb::array_view <const gdb_byte> (value_contents (arg1),
+							 TYPE_LENGTH (type1)),
 		       type_byte_order (type1), type1->is_unsigned (),
 		       fixed_point_scaling_factor (type1));
-  v2.read_fixed_point (value_contents (arg2), TYPE_LENGTH (type2),
+  v2.read_fixed_point (gdb::array_view <const gdb_byte> (value_contents (arg2),
+							 TYPE_LENGTH (type2)),
 		       type_byte_order (type2), type2->is_unsigned (),
 		       fixed_point_scaling_factor (type2));
 
@@ -950,7 +952,8 @@ fixed_point_binop (struct value *arg1, struct value *arg2, enum exp_opcode op)
   do { \
       val = allocate_value (type1); \
       (RESULT).write_fixed_point			\
-        (value_contents_raw (val), TYPE_LENGTH (type1), \
+        (gdb::array_view <gdb_byte> (value_contents_raw (val), \
+				     TYPE_LENGTH (type1)), \
 	 type_byte_order (type1), type1->is_unsigned (), \
 	 fixed_point_scaling_factor (type1)); \
      } while (0)
diff --git a/gdb/valops.c b/gdb/valops.c
index d63f233..b0b1209 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -357,7 +357,8 @@ value_cast_to_fixed_point (struct type *to_type, struct value *from_val)
     {
       gdb_mpz vz;
 
-      vz.read (value_contents (from_val), TYPE_LENGTH (from_type),
+      vz.read (gdb::array_view<const gdb_byte> (value_contents (from_val),
+						TYPE_LENGTH (from_type)),
 	       type_byte_order (from_type), from_type->is_unsigned ());
       mpq_set_z (vq.val, vz.val);
 
@@ -378,8 +379,9 @@ value_cast_to_fixed_point (struct type *to_type, struct value *from_val)
   /* Finally, create the result value, and pack the unscaled value
      in it.  */
   struct value *result = allocate_value (to_type);
-  unscaled.write (value_contents_raw (result),
-		  TYPE_LENGTH (to_type), type_byte_order (to_type),
+  unscaled.write (gdb::array_view<gdb_byte> (value_contents_raw (result),
+					     TYPE_LENGTH (to_type)),
+		  type_byte_order (to_type),
 		  to_type->is_unsigned ());
 
   return result;
@@ -523,7 +525,8 @@ value_cast (struct type *type, struct value *arg2)
 	  gdb_mpq fp_val;
 
 	  fp_val.read_fixed_point
-	    (value_contents (arg2), TYPE_LENGTH (type2),
+	    (gdb::array_view<const gdb_byte> (value_contents (arg2),
+					      TYPE_LENGTH (type2)),
 	     type_byte_order (type2), type2->is_unsigned (),
 	     fixed_point_scaling_factor (type2));
 
diff --git a/gdb/valprint.c b/gdb/valprint.c
index abef002..21a4bf4 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -809,7 +809,8 @@ generic_val_print_fixed_point (struct value *val, struct ui_file *stream,
       const gdb_byte *valaddr = value_contents_for_printing (val);
       gdb_mpf f;
 
-      f.read_fixed_point (valaddr, TYPE_LENGTH (type),
+      f.read_fixed_point (gdb::array_view<const gdb_byte> (valaddr,
+							   TYPE_LENGTH (type)),
 			  type_byte_order (type), type->is_unsigned (),
 			  fixed_point_scaling_factor (type));
 
diff --git a/gdb/value.c b/gdb/value.c
index 3b207cd..bff4afc 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2812,7 +2812,8 @@ unpack_long (struct type *type, const gdb_byte *valaddr)
     case TYPE_CODE_FIXED_POINT:
       {
 	gdb_mpq vq;
-	vq.read_fixed_point (valaddr, len, byte_order, nosign,
+	vq.read_fixed_point (gdb::array_view<const gdb_byte> (valaddr, len),
+			     byte_order, nosign,
 			     fixed_point_scaling_factor (type));
 
 	gdb_mpz vz;
-- 
2.1.4