[02/20] Rename struct attribute accessors

Message ID 20200328192208.11324-3-tom@tromey.com
State Superseded
Headers show
Series
  • Make DWARF attribute references safe
Related show

Commit Message

Tom Tromey March 28, 2020, 7:21 p.m.
This removes the "value_as_" prefix from the struct value accessors.
This seemed unnecessarily wordy to me.

gdb/ChangeLog
2020-03-28  Tom Tromey  <tom@tromey.com>

	* dwarf2/read.c (dwarf2_find_base_address, read_call_site_scope)
	(dwarf2_get_pc_bounds, dwarf2_record_block_ranges)
	(partial_die_info::read, dwarf2_string_attr, new_symbol): Update.
	* dwarf2/attribute.h (struct attribute): Rename methods.
	* dwarf2/attribute.c (attribute::address): Rename from
	value_as_address.
	(attribute::string): Rename from value_as_string.
---
 gdb/ChangeLog          | 10 ++++++++++
 gdb/dwarf2/attribute.c |  4 ++--
 gdb/dwarf2/attribute.h |  4 ++--
 gdb/dwarf2/read.c      | 24 ++++++++++++------------
 4 files changed, 26 insertions(+), 16 deletions(-)

-- 
2.17.2

Comments

Tankut Baris Aktemur via Gdb-patches March 30, 2020, 8:58 a.m. | #1
On Saturday, March 28, 2020 8:22 PM, Tom Tromey wrote:
> This removes the "value_as_" prefix from the struct value accessors.

> This seemed unnecessarily wordy to me.

> 

> gdb/ChangeLog

> 2020-03-28  Tom Tromey  <tom@tromey.com>

> 

> 	* dwarf2/read.c (dwarf2_find_base_address, read_call_site_scope)

> 	(dwarf2_get_pc_bounds, dwarf2_record_block_ranges)

> 	(partial_die_info::read, dwarf2_string_attr, new_symbol): Update.

> 	* dwarf2/attribute.h (struct attribute): Rename methods.

> 	* dwarf2/attribute.c (attribute::address): Rename from

> 	value_as_address.

> 	(attribute::string): Rename from value_as_string.

> ---

>  gdb/ChangeLog          | 10 ++++++++++

>  gdb/dwarf2/attribute.c |  4 ++--

>  gdb/dwarf2/attribute.h |  4 ++--

>  gdb/dwarf2/read.c      | 24 ++++++++++++------------

>  4 files changed, 26 insertions(+), 16 deletions(-)

> 

> diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c

> index a4a6db76c8f..1bdd4cf7abb 100644

> --- a/gdb/dwarf2/attribute.c

> +++ b/gdb/dwarf2/attribute.c

> @@ -32,7 +32,7 @@

>  /* See attribute.h.  */

> 

>  CORE_ADDR

> -attribute::value_as_address () const

> +attribute::address () const

>  {

>    CORE_ADDR addr;

> 

> @@ -62,7 +62,7 @@ attribute::value_as_address () const

>  /* See attribute.h.  */

> 

>  const char *

> -attribute::value_as_string () const

> +attribute::string () const

>  {

>    if (form == DW_FORM_strp || form == DW_FORM_line_strp

>        || form == DW_FORM_string

> diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h

> index a9cabd69f9f..cefd3c5541e 100644

> --- a/gdb/dwarf2/attribute.h

> +++ b/gdb/dwarf2/attribute.h

> @@ -44,11 +44,11 @@ struct attribute

>  {

>    /* Read the given attribute value as an address, taking the

>       attribute's form into account.  */

> -  CORE_ADDR value_as_address () const;

> +  CORE_ADDR address () const;

> 

>    /* If the attribute has a string form, return the string value;

>       otherwise return NULL.  */

> -  const char *value_as_string () const;

> +  const char *string () const;

> 

>    /* Return non-zero if ATTR's value is a section offset --- classes

>       lineptr, loclistptr, macptr or rangelistptr --- or zero, otherwise.

> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c

> index 98323f8920b..d2b274a6e3a 100644

> --- a/gdb/dwarf2/read.c

> +++ b/gdb/dwarf2/read.c

> @@ -5766,12 +5766,12 @@ dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)

> 

>    attr = dwarf2_attr (die, DW_AT_entry_pc, cu);

>    if (attr != nullptr)

> -    cu->base_address = attr->value_as_address ();

> +    cu->base_address = attr->address ();

>    else

>      {

>        attr = dwarf2_attr (die, DW_AT_low_pc, cu);

>        if (attr != nullptr)

> -	cu->base_address = attr->value_as_address ();

> +	cu->base_address = attr->address ();

>      }

>  }

> 

> @@ -13022,7 +13022,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)

>  		 sect_offset_str (die->sect_off), objfile_name (objfile));

>        return;

>      }

> -  pc = attr->value_as_address () + baseaddr;

> +  pc = attr->address () + baseaddr;

>    pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);

> 

>    if (cu->call_site_htab == NULL)

> @@ -13724,8 +13724,8 @@ dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,

>        attr = dwarf2_attr (die, DW_AT_low_pc, cu);

>        if (attr != nullptr)

>          {

> -	  low = attr->value_as_address ();

> -	  high = attr_high->value_as_address ();

> +	  low = attr->address ();

> +	  high = attr_high->address ();

>  	  if (cu->header.version >= 4 && attr_high->form_is_constant ())

>  	    high += low;

>  	}

> @@ -13897,8 +13897,8 @@ dwarf2_record_block_ranges (struct die_info *die, struct block *block,

>        attr = dwarf2_attr (die, DW_AT_low_pc, cu);

>        if (attr != nullptr)

>          {

> -          CORE_ADDR low = attr->value_as_address ();

> -	  CORE_ADDR high = attr_high->value_as_address ();

> +          CORE_ADDR low = attr->address ();


The existing code had 8 spaces that can now be replaced with a tab.

Thanks.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Simon Marchi March 30, 2020, 2:45 p.m. | #2
On 2020-03-28 3:21 p.m., Tom Tromey wrote:
> This removes the "value_as_" prefix from the struct value accessors.

> This seemed unnecessarily wordy to me.


I would perhaps use "as_string" and "as_address" rather than just
"string" and "address".  At least for "address", it could be
interpreted by some as returning the address (pointer) of the
attribute object.  The "as_" prefix makes me naturally read it as
"interpret this attribute as a string", "interpret this attribute as
an address".

Not a big deal though, I'm also fine with the patch as-is.

Simon
Tom Tromey March 30, 2020, 11:39 p.m. | #3
>>>>> ">" == Aktemur, Tankut Baris <tankut.baris.aktemur@intel.com> writes:


>> -          CORE_ADDR low = attr->value_as_address ();

>> -	  CORE_ADDR high = attr_high->value_as_address ();

>> +          CORE_ADDR low = attr->address ();


>> The existing code had 8 spaces that can now be replaced with a tab.


Thanks, I changed this.

Tom
Tom Tromey March 30, 2020, 11:39 p.m. | #4
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:


Simon> I would perhaps use "as_string" and "as_address" rather than just
Simon> "string" and "address".

I considered this but didn't do it.  But, I'm making the change now.

Tom

Patch

diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c
index a4a6db76c8f..1bdd4cf7abb 100644
--- a/gdb/dwarf2/attribute.c
+++ b/gdb/dwarf2/attribute.c
@@ -32,7 +32,7 @@ 
 /* See attribute.h.  */
 
 CORE_ADDR
-attribute::value_as_address () const
+attribute::address () const
 {
   CORE_ADDR addr;
 
@@ -62,7 +62,7 @@  attribute::value_as_address () const
 /* See attribute.h.  */
 
 const char *
-attribute::value_as_string () const
+attribute::string () const
 {
   if (form == DW_FORM_strp || form == DW_FORM_line_strp
       || form == DW_FORM_string
diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h
index a9cabd69f9f..cefd3c5541e 100644
--- a/gdb/dwarf2/attribute.h
+++ b/gdb/dwarf2/attribute.h
@@ -44,11 +44,11 @@  struct attribute
 {
   /* Read the given attribute value as an address, taking the
      attribute's form into account.  */
-  CORE_ADDR value_as_address () const;
+  CORE_ADDR address () const;
 
   /* If the attribute has a string form, return the string value;
      otherwise return NULL.  */
-  const char *value_as_string () const;
+  const char *string () const;
 
   /* Return non-zero if ATTR's value is a section offset --- classes
      lineptr, loclistptr, macptr or rangelistptr --- or zero, otherwise.
diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 98323f8920b..d2b274a6e3a 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -5766,12 +5766,12 @@  dwarf2_find_base_address (struct die_info *die, struct dwarf2_cu *cu)
 
   attr = dwarf2_attr (die, DW_AT_entry_pc, cu);
   if (attr != nullptr)
-    cu->base_address = attr->value_as_address ();
+    cu->base_address = attr->address ();
   else
     {
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr != nullptr)
-	cu->base_address = attr->value_as_address ();
+	cu->base_address = attr->address ();
     }
 }
 
@@ -13022,7 +13022,7 @@  read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu)
 		 sect_offset_str (die->sect_off), objfile_name (objfile));
       return;
     }
-  pc = attr->value_as_address () + baseaddr;
+  pc = attr->address () + baseaddr;
   pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc);
 
   if (cu->call_site_htab == NULL)
@@ -13724,8 +13724,8 @@  dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr != nullptr)
         {
-	  low = attr->value_as_address ();
-	  high = attr_high->value_as_address ();
+	  low = attr->address ();
+	  high = attr_high->address ();
 	  if (cu->header.version >= 4 && attr_high->form_is_constant ())
 	    high += low;
 	}
@@ -13897,8 +13897,8 @@  dwarf2_record_block_ranges (struct die_info *die, struct block *block,
       attr = dwarf2_attr (die, DW_AT_low_pc, cu);
       if (attr != nullptr)
         {
-          CORE_ADDR low = attr->value_as_address ();
-	  CORE_ADDR high = attr_high->value_as_address ();
+          CORE_ADDR low = attr->address ();
+	  CORE_ADDR high = attr_high->address ();
 
 	  if (cu->header.version >= 4 && attr_high->form_is_constant ())
 	    high += low;
@@ -17896,15 +17896,15 @@  partial_die_info::read (const struct die_reader_specs *reader,
 	  /* Note that both forms of linkage name might appear.  We
 	     assume they will be the same, and we only store the last
 	     one we see.  */
-	  linkage_name = attr.value_as_string ();
+	  linkage_name = attr.string ();
 	  break;
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
-	  lowpc = attr.value_as_address ();
+	  lowpc = attr.address ();
 	  break;
 	case DW_AT_high_pc:
 	  has_high_pc_attr = 1;
-	  highpc = attr.value_as_address ();
+	  highpc = attr.address ();
 	  if (cu->header.version >= 4 && attr.form_is_constant ())
 		high_pc_relative = 1;
 	  break;
@@ -18949,7 +18949,7 @@  dwarf2_string_attr (struct die_info *die, unsigned int name, struct dwarf2_cu *c
 
   if (attr != NULL)
     {
-      str = attr->value_as_string ();
+      str = attr->string ();
       if (str == nullptr)
         complaint (_("string type expected for attribute %s for "
 		     "DIE at %s in module %s"),
@@ -20076,7 +20076,7 @@  new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu,
 	    {
 	      CORE_ADDR addr;
 
-	      addr = attr->value_as_address ();
+	      addr = attr->address ();
 	      addr = gdbarch_adjust_dwarf2_addr (gdbarch, addr + baseaddr);
 	      SET_SYMBOL_VALUE_ADDRESS (sym, addr);
 	    }