diff mbox series

[08/10] gdb: have value_as_address call unpack_pointer

Message ID 312bba523f1d5701df37f3c7984c1b18fc4b5f9d.1678460067.git.aburgess@redhat.com
State New
Headers
Series Improvements & Cleanup For Python Unwinder API |

Commit Message

Andrew Burgess March 10, 2023, 2:55 p.m. UTC 
  While refactoring some other code in gdb/python/* I wanted to merge
two code paths.  One path calls value_as_address, while the other
calls unpack_pointer.

I suspect calling value_as_address is the correct choice, but, while
examining the code I noticed that value_as_address calls unpack_long
rather than unpack_pointer.

Under the hood, unpack_pointer does just call unpack_long so there's
no real difference here, but it feels like value_as_address should
call unpack_pointer.

I've updated the code to use unpack_pointer, and changed a related
comment to say that we call unpack_pointer.  I've also adjusted the
header comment on value_as_address.  The existing header refers to
some code that is now commented out.

Rather than trying to describe the whole algorithm of
value_as_address, which is already well commented within the function,
I've just trimmed the comment on value_as_address to be a brief
summary of what the function does.

There should be no user visible changes after this commit.
---
 gdb/value.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Tom Tromey March 10, 2023, 3:28 p.m. UTC | #1 
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> +/* Extract a value as a C pointer.  Does not deallocate the value.  */

I think that 'deallocate' sentence can be removed.
I don't know when that was last relevant, values are essentially never
deallocated by hand.

Tom
Andrew Burgess March 10, 2023, 10:08 p.m. UTC | #2 
Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Andrew> +/* Extract a value as a C pointer.  Does not deallocate the value.  */
>
> I think that 'deallocate' sentence can be removed.
> I don't know when that was last relevant, values are essentially never
> deallocated by hand.

Thanks, have updated the comment in my local tree to read:

  /* Extract a value as a C pointer.  */

Thanks,
Andrew
diff mbox series

Patch

diff --git a/gdb/value.c b/gdb/value.c
index 7b4df338304..d432b29b61b 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2551,9 +2551,8 @@  value_as_long (struct value *val)
   return unpack_long (val->type (), val->contents ().data ());
 }
 
-/* Extract a value as a C pointer.  Does not deallocate the value.
-   Note that val's type may not actually be a pointer; value_as_long
-   handles all the cases.  */
+/* Extract a value as a C pointer.  Does not deallocate the value.  */
+
 CORE_ADDR
 value_as_address (struct value *val)
 {
@@ -2592,7 +2591,7 @@  value_as_address (struct value *val)
      to COERCE_ARRAY below actually does all the usual unary
      conversions, which includes converting values of type `function'
      to `pointer to function'.  This is the challenging conversion
-     discussed above.  Then, `unpack_long' will convert that pointer
+     discussed above.  Then, `unpack_pointer' will convert that pointer
      back into an address.
 
      So, suppose the user types `disassemble foo' on an architecture
@@ -2653,7 +2652,7 @@  value_as_address (struct value *val)
     return gdbarch_integer_to_address (gdbarch, val->type (),
 				       val->contents ().data ());
 
-  return unpack_long (val->type (), val->contents ().data ());
+  return unpack_pointer (val->type (), val->contents ().data ());
 #endif
 }