Re: [PATCH 6/7] jit: c++-ify gdb_block

Christian Biesinger via gdb-patches
2019-12-13T15:11:13Z

On Fri, Dec 13, 2019, 01:04 Simon Marchi <simon.marchi@polymtl.ca> wrote:

> Add a constructor to gdb_block, change the name field to be a
> gdb::unique_xmalloc_ptr.
>
> gdb/ChangeLog:
>
> * jit.c (struct gdb_block): Add constructor, initialize
> real_block field.
> <name): Change type to gdb::unique_xmalloc_ptr.
> (struct gdb_symtab) <~gdb_symtab>: Free blocks with delete.
> (jit_block_open_impl): Use gdb_block constructor.
> (finalize_symtab): Adjust to gdb::unique_xmalloc_ptr.
> ---
> gdb/jit.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/jit.c b/gdb/jit.c
> index 1f6de6796e10..56a51f04eb2f 100644
> --- a/gdb/jit.c
> +++ b/gdb/jit.c
> @@ -428,20 +428,28 @@ jit_read_code_entry (struct gdbarch *gdbarch,
>
> struct gdb_block
> {
> + gdb_block (gdb_block *parent, CORE_ADDR begin, CORE_ADDR end,
> + const char *name)
> + : parent (parent),
> + begin (begin),
> + end (end),
> + name (name != nullptr ? xstrdup (name) : nullptr)
> + {}
> +
> /* The parent of this block. */
> struct gdb_block *parent;
>
> /* Points to the "real" block that is being built out of this
> instance. This block will be added to a blockvector, which will
> then be added to a symtab. */
> - struct block *real_block;
> + struct block *real_block = nullptr;
>
> /* The first and last code address corresponding to this block. */
> CORE_ADDR begin, end;
>
> /* The name of this block (if any). If this is non-NULL, the
> FUNCTION symbol symbol is set to this value. */
> - const char *name;
> + gdb::unique_xmalloc_ptr<char> name;
> };
>
> /* Proxy object for building a symtab. */
> @@ -455,10 +463,7 @@ struct gdb_symtab
> ~gdb_symtab ()
> {
> for (gdb_block *gdb_block_iter : this->blocks)
> - {
> - xfree ((void *) gdb_block_iter->name);
> - xfree (gdb_block_iter);
> - }
> + delete gdb_block_iter;
> }
>

Have you considered making this a vector of unique_ptrs, so you don't have
to manually delete them?

> /* The list of blocks in this symtab. These will eventually be
> @@ -534,16 +539,9 @@ jit_block_open_impl (struct gdb_symbol_callbacks *cb,
> struct gdb_symtab *symtab, struct gdb_block *parent,
> GDB_CORE_ADDR begin, GDB_CORE_ADDR end, const char
> *name)
> {
> - struct gdb_block *block = XCNEW (struct gdb_block);
> -
> - block->begin = (CORE_ADDR) begin;
> - block->end = (CORE_ADDR) end;
> - block->name = name ? xstrdup (name) : NULL;
> - block->parent = parent;
> -
> /* Place the block at the end of the vector, it will be sorted when the
> symtab is finalized. */
> - symtab->blocks.push_back (block);
> + symtab->blocks.push_back (new gdb_block (parent, begin, end, name));
> return symtab->blocks.back ();
> }
>
> @@ -665,7 +663,7 @@ finalize_symtab (struct gdb_symtab *stab, struct
> objfile *objfile)
> SYMBOL_BLOCK_VALUE (block_name) = new_block;
>
> block_name->name = obstack_strdup (&objfile->objfile_obstack,
> - gdb_block_iter->name);
> + gdb_block_iter->name.get ());
>
> BLOCK_FUNCTION (new_block) = block_name;
>
> --
> 2.24.1
>
>