Re: [PATCH 1/2] Remove munmap_listp_free_cleanup

[Simon Marchi <simark at simark dot ca>]

On 2018-09-15 6:24 p.m., Tom Tromey wrote:
> diff --git a/gdb/compile/compile-object-load.h b/gdb/compile/compile-object-load.h
> index 6f62535878a..7569c42bf5c 100644
> --- a/gdb/compile/compile-object-load.h
> +++ b/gdb/compile/compile-object-load.h
> @@ -18,8 +18,31 @@
>  #define GDB_COMPILE_OBJECT_LOAD_H
>  
>  #include "compile-internal.h"
> +#include <list>
>  
> -struct munmap_list;
> +struct munmap_list
> +{
> +public:
> +
> +  munmap_list () = default;
> +  ~munmap_list ();
> +
> +  DISABLE_COPY_AND_ASSIGN (munmap_list);
> +
> +  /* Add a region to the list.  */
> +  void add (CORE_ADDR addr, CORE_ADDR size);
> +
> +private:
> +
> +  /* Track inferior memory reserved by inferior mmap.  */
> +
> +  struct munmap_item
> +  {
> +    CORE_ADDR addr, size;
> +  };
> +
> +  std::list<munmap_item> items;

Not a big deal, but we might as well use a vector.  The objects are cheap to copy, so there's probably less overhead overall (memory and time) to use a vector than a doubly linked list.

I was going to say:

1. Why not use munmap_list as directly as field of setup_sections_data, instead
   of a unique_ptr, and
2. std::move it to the compile_module object.

But that would require C++ifying compile_module and probably do_module_cleanup too,
and that's perhaps a too big of a step for this patch.

Simon