Re: [RFA 1/3] C++-ify prologue-value's pv_area

[Simon Marchi <simark at simark dot ca>]

On 2017-10-08 01:07 PM, Tom Tromey wrote:
> diff --git a/gdb/prologue-value.c b/gdb/prologue-value.c
> index 5263f131ad..b830b8f50d 100644
> --- a/gdb/prologue-value.c
> +++ b/gdb/prologue-value.c
> @@ -278,7 +278,7 @@ pv_is_array_ref (pv_t addr, CORE_ADDR size,
>     The entry with the lowest offset simply follows the entry with the
>     highest offset.  Entries may abut, but never overlap.  The area's
>     'entry' pointer points to an arbitrary node in the ring.  */
> -struct area_entry
> +struct pv_area::area_entry
>  {
>    /* Links in the doubly-linked ring.  */
>    struct area_entry *prev, *next;
> @@ -296,44 +296,19 @@ struct area_entry
>  };
>  
>  
> -struct pv_area
> +pv_area::pv_area (int base_reg, int addr_bit)
> +  : m_base_reg (base_reg),
> +    /* Remember that shift amounts equal to the type's width are
> +       undefined.  */
> +    m_addr_mask (((((CORE_ADDR) 1 << (addr_bit - 1)) - 1) << 1) | 1),
> +    m_entry (nullptr)
>  {
> -  /* This area's base register.  */
> -  int base_reg;
> -
> -  /* The mask to apply to addresses, to make the wrap-around happen at
> -     the right place.  */
> -  CORE_ADDR addr_mask;
> -
> -  /* An element of the doubly-linked ring of entries, or zero if we
> -     have none.  */
> -  struct area_entry *entry;
> -};
> -
> -
> -struct pv_area *
> -make_pv_area (int base_reg, int addr_bit)
> -{
> -  struct pv_area *a = XNEW (struct pv_area);
> -
> -  memset (a, 0, sizeof (*a));
> -
> -  a->base_reg = base_reg;
> -  a->entry = 0;
> -
> -  /* Remember that shift amounts equal to the type's width are
> -     undefined.  */
> -  a->addr_mask = ((((CORE_ADDR) 1 << (addr_bit - 1)) - 1) << 1) | 1;
> -
> -  return a;
>  }
>  
> -
> -/* Delete all entries from AREA.  */
> -static void
> -clear_entries (struct pv_area *area)
> +void
> +pv_area::clear_entries ()
>  {
> -  struct area_entry *e = area->entry;
> +  struct area_entry *e = m_entry;
>  
>    if (e)
>      {
> @@ -347,37 +322,21 @@ clear_entries (struct pv_area *area)
>            xfree (e);
>            e = next;
>          }
> -      while (e != area->entry);
> +      while (e != m_entry);
>  
> -      area->entry = 0;
> +      m_entry = 0;
>      }
>  }
>  
>  
> -void
> -free_pv_area (struct pv_area *area)
> +pv_area::~pv_area ()
>  {
> -  clear_entries (area);
> -  xfree (area);
> -}
> -
> -
> -static void
> -do_free_pv_area_cleanup (void *arg)
> -{
> -  free_pv_area ((struct pv_area *) arg);
> -}
> -
> -
> -struct cleanup *
> -make_cleanup_free_pv_area (struct pv_area *area)
> -{
> -  return make_cleanup (do_free_pv_area_cleanup, (void *) area);
> +  clear_entries ();
>  }
>  
>  
>  int
> -pv_area_store_would_trash (struct pv_area *area, pv_t addr)
> +pv_area::store_would_trash (pv_t addr)
>  {

On top of methods, I think there should be

  /* See prologue-value.h.  */

like for regular functions.

Otherwise, LGTM.

Simon