Re: [PATCH 2/5] Update comments in struct value for non-8-bits architectures

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 15-07-24 07:27 AM, Pedro Alves wrote:
> On 07/16/2015 07:51 PM, Simon Marchi wrote:
>> gdb/ChangeLog:
>>
>> 	* value.c (struct value): Update comments.
> 
> Looks good to me, though as mentioned in the other patch, I think
> these comments should be explicit in saying "host" and "target".  These are
> central structures that people study first, and being crystal clear
> should help grasp the byte vs memory units concepts sooner.
> 
> Thanks,
> Pedro Alves

Updated version:


>From 4441bdcc4e55d2397d2ca7918669a1ccf1676a25 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 2 Jul 2015 17:52:40 -0400
Subject: [PATCH] Update comments in struct value for non-8-bits architectures

gdb/ChangeLog:

	* value.c (struct value): Update comments.
---
 gdb/value.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/gdb/value.c b/gdb/value.c
index 4399493..7fb7e2b 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -234,11 +234,11 @@ struct value
     } computed;
   } location;

-  /* Describes offset of a value within lval of a structure in bytes.
-     If lval == lval_memory, this is an offset to the address.  If
-     lval == lval_register, this is a further offset from
-     location.address within the registers structure.  Note also the
-     member embedded_offset below.  */
+  /* Describes offset of a value within lval of a structure in target
+     addressable memory units.  If lval == lval_memory, this is an offset to
+     the address.  If lval == lval_register, this is a further offset from
+     location.address within the registers structure.  Note also the member
+     embedded_offset below.  */
   int offset;

   /* Only used for bitfields; number of bits contained in them.  */
@@ -291,19 +291,19 @@ struct value

      When we store the entire object, `enclosing_type' is the run-time
      type -- the complete object -- and `embedded_offset' is the
-     offset of `type' within that larger type, in bytes.  The
-     value_contents() macro takes `embedded_offset' into account, so
-     most GDB code continues to see the `type' portion of the value,
-     just as the inferior would.
+     offset of `type' within that larger type, in target addressable memory
+     units.  The value_contents() macro takes `embedded_offset' into account,
+     so most GDB code continues to see the `type' portion of the value, just
+     as the inferior would.

      If `type' is a pointer to an object, then `enclosing_type' is a
      pointer to the object's run-time type, and `pointed_to_offset' is
-     the offset in bytes from the full object to the pointed-to object
-     -- that is, the value `embedded_offset' would have if we followed
-     the pointer and fetched the complete object.  (I don't really see
-     the point.  Why not just determine the run-time type when you
-     indirect, and avoid the special case?  The contents don't matter
-     until you indirect anyway.)
+     the offset in target addressable memory units from the full object
+     to the pointed-to object -- that is, the value `embedded_offset' would
+     have if we followed the pointer and fetched the complete object.
+     (I don't really see the point.  Why not just determine the
+     run-time type when you indirect, and avoid the special case?  The
+     contents don't matter until you indirect anyway.)

      If we're not doing anything fancy, `enclosing_type' is equal to
      `type', and `embedded_offset' is zero, so everything works
-- 
2.1.4