Re: [PATCH 3/6] Class-fy regcache

[Pedro Alves <palves@redhat.com>]

On 04/25/2017 09:28 PM, Yao Qi wrote:
> This patch moves regcache declaration to regcache.h, and converts
> regcache apis to member functions, for example, regcache_invalidate
> is changed to regcache::invalidate.  Some changes are not done in
> this patch,
> 
>  - keep "struct regcache" instead of renaming to "class regcache",
>    because "struct regcache" is widely used (even in GDBserver).

"struct regcache *" and "class regcache *" are interchangeble,
they mean exactly the same thing.

> +
> +/* Dump the contents of a register from the register cache to the target
> +   debug.  */
> +  void debug_print_register (const char *func, int regno);
> +
> +private:
> +  explicit regcache (struct gdbarch *gdbarch, struct address_space *aspace_,
> +		     bool readonly_p_);
> +
> +  gdb_byte *register_buffer (int regnum) const;
> +
> +  void
> +  restore (struct regcache *src);
> +
> +  void cpy_no_passthrough (struct regcache *src);
> +
> +  enum register_status
> +  xfer_part (int regnum, int offset, int len, void *in, const void *out,
> +	     enum register_status (*read) (struct regcache *regcache,
> +					   int regnum, gdb_byte *buf),
> +	     void (*write) (struct regcache *regcache, int regnum,
> +			    const gdb_byte *buf));
> +


"restore" and "xfer_part" stand out as having odd formatting with
the type on its own line.

> +  void transfer_regset (const struct regset *regset,
> +			struct regcache *out_regcache,
> +			int regnum, const void *in_buf,
> +			void *out_buf, size_t size) const;
> +
> +  struct regcache_descr *descr;
> +
> +  /* The address space of this register cache (for registers where it
> +     makes sense, like PC or SP).  */
> +  struct address_space *aspace;
> +
> +  /* The register buffers.  A read-only register cache can hold the
> +     full [0 .. gdbarch_num_regs + gdbarch_num_pseudo_regs) while a read/write
> +     register cache can only hold [0 .. gdbarch_num_regs).  */
> +  gdb_byte *registers;
> +  /* Register cache status.  */
> +  signed char *register_status;
> +  /* Is this a read-only cache?  A read-only cache is used for saving
> +     the target's register state (e.g, across an inferior function
> +     call or just before forcing a function return).  A read-only
> +     cache can only be updated via the methods regcache_dup() and
> +     regcache_cpy().  The actual contents are determined by the
> +     reggroup_save and reggroup_restore methods.  */
> +  bool readonly_p;
> +  /* If this is a read-write cache, which thread's registers is
> +     it connected to?  */
> +  ptid_t ptid;

All these fields should be prefixed with "m_" now.  You already
need to touch all code that references these fields, so it seems to
me that the patch would have the exact same size if you do it
at the same time.  That allow renaming the
get_ptid() method to ptid(), "get_aspace()" to "aspace()" etc. too.

Thanks,
Pedro Alves