Re: [PATCH v2 04/11] gdbserver: convert init_register_cache and new_register_cache into constructors

[Simon Marchi <simark@simark.ca>]

> @@ -112,55 +112,38 @@ regcache_invalidate (void)
>  
>  #endif
>  
> -struct regcache *
> -init_register_cache (struct regcache *regcache,
> -		     const struct target_desc *tdesc,
> -		     unsigned char *regbuf)
> +regcache::regcache (const target_desc *tdesc,
> +		    unsigned char *regbuf)
>  {
> -  if (regbuf == NULL)
> -    {
> -#ifndef IN_PROCESS_AGENT
> -      /* Make sure to zero-initialize the register cache when it is
> -	 created, in case there are registers the target never
> -	 fetches.  This way they'll read as zero instead of
> -	 garbage.  */
> -      regcache->tdesc = tdesc;
> -      regcache->registers
> -	= (unsigned char *) xcalloc (1, tdesc->registers_size);
> -      regcache->registers_owned = true;
> -      regcache->register_status
> -	= (unsigned char *) xmalloc (tdesc->reg_defs.size ());
> -      memset ((void *) regcache->register_status, REG_UNAVAILABLE,
> -	      tdesc->reg_defs.size ());
> -#else
> -      gdb_assert_not_reached ("can't allocate memory from the heap");
> -#endif
> -    }
> -  else
> -    {
> -      regcache->tdesc = tdesc;
> -      regcache->registers = regbuf;
> -      regcache->registers_owned = false;
> +  gdb_assert (regbuf != nullptr);
> +
> +  this->tdesc = tdesc;
> +  this->registers = regbuf;
> +  this->registers_owned = false;
>  #ifndef IN_PROCESS_AGENT
> -      regcache->register_status = NULL;
> +  this->register_status = nullptr;
>  #endif

These could be initialized in an initialization list.

> -    }
> -
> -  regcache->registers_fetched = false;
> -
> -  return regcache;
> +  this->registers_fetched = false;
>  }
>  
>  #ifndef IN_PROCESS_AGENT
>  
> -struct regcache *
> -new_register_cache (const struct target_desc *tdesc)
> +regcache::regcache (const target_desc *tdesc)
>  {
> -  struct regcache *regcache = new struct regcache;
> -
>    gdb_assert (tdesc->registers_size != 0);
>  
> -  return init_register_cache (regcache, tdesc, NULL);
> +  /* Make sure to zero-initialize the register cache when it is
> +     created, in case there are registers the target never
> +     fetches.  This way they'll read as zero instead of
> +     garbage.  */
> +  this->tdesc = tdesc;
> +  this->registers
> +    = (unsigned char *) xcalloc (1, tdesc->registers_size);
> +  this->registers_owned = true;
> +  this->register_status
> +    = (unsigned char *) xmalloc (tdesc->reg_defs.size ());

Same here.

> diff --git a/gdbserver/regcache.h b/gdbserver/regcache.h
> index 12345a3439cd7dedc70cd06f13450dde58214124..ea6c26a37a07bf6d336d6bdf327d543fae0b6370 100644
> --- a/gdbserver/regcache.h
> +++ b/gdbserver/regcache.h
> @@ -45,7 +45,13 @@ struct regcache : public reg_buffer_common
>  #ifndef IN_PROCESS_AGENT
>    /* One of REG_UNAVAILABLE or REG_VALID.  */
>    unsigned char *register_status = nullptr;
> +
> +  /* Constructors.  */
> +  regcache (const target_desc *tdesc);
>  #endif
> +  regcache (const target_desc *tdesc, unsigned char *regbuf);

I don't think the "Constructors" is that helpful.  On the other hand, I
would appreciate a comment saying what the constructor with `regbuf`
does differently from the other one (I guess use that buffer to store
register data instead of dynamically allocating?).

> diff --git a/gdbserver/tracepoint.cc b/gdbserver/tracepoint.cc
> index 1b6e9d05273ea7a709f1880473c070c59169ddd5..9a00d1f272f905cadbc87265ed0be3d87f7b54df 100644
> --- a/gdbserver/tracepoint.cc
> +++ b/gdbserver/tracepoint.cc
> @@ -1288,7 +1288,7 @@ struct tracepoint_hit_ctx
>  struct fast_tracepoint_ctx : public tracepoint_hit_ctx
>  {
>    fast_tracepoint_ctx (unsigned char *_regs)
> -    : regcache_initted (0), regs (_regs)
> +    : regcache (), regs (_regs)

`regcache ()` becomes unnecessary.

Simon