Re: [Qemu-devel] [PATCH 4/5] tests/libqos: embed allocators instead of malloc-ing them separately

[Thomas Huth]

On 2019-01-15 19:19, Paolo Bonzini wrote:
> qgraph will embed these objects instead of allocating them in a separate
> object.  Expose a new API "generic_alloc_init" and "generic_alloc_destroy"
> for that, and rename the existing API with s/init/new/ and s/uninit/free/.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
[...]
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> index f7bae47..d44ae27 100644
> --- a/tests/libqos/malloc.c
> +++ b/tests/libqos/malloc.c
> @@ -15,24 +15,12 @@
>  #include "qemu-common.h"
>  #include "qemu/host-utils.h"
>  
> -typedef QTAILQ_HEAD(MemList, MemBlock) MemList;
> -
>  typedef struct MemBlock {
>      QTAILQ_ENTRY(MemBlock) MLIST_ENTNAME;
>      uint64_t size;
>      uint64_t addr;
>  } MemBlock;
>  
> -struct QGuestAllocator {
> -    QAllocOpts opts;
> -    uint64_t start;
> -    uint64_t end;
> -    uint32_t page_size;
> -
> -    MemList *used;
> -    MemList *free;
> -};
> -
>  #define DEFAULT_PAGE_SIZE 4096
>  
>  static void mlist_delete(MemList *list, MemBlock *node)
> @@ -225,7 +213,7 @@ static void mlist_free(QGuestAllocator *s, uint64_t addr)
>   * Mostly for valgrind happiness, but it does offer
>   * a chokepoint for debugging guest memory leaks, too.
>   */
> -void alloc_uninit(QGuestAllocator *allocator)
> +void alloc_destroy(QGuestAllocator *allocator)
>  {
>      MemBlock *node;
>      MemBlock *tmp;
> @@ -261,7 +249,6 @@ void alloc_uninit(QGuestAllocator *allocator)
>  
>      g_free(allocator->used);
>      g_free(allocator->free);
> -    g_free(allocator);
>  }
>  
>  uint64_t guest_alloc(QGuestAllocator *allocator, size_t size)
> @@ -297,9 +284,10 @@ void guest_free(QGuestAllocator *allocator, uint64_t 
> addr)
>      }
>  }
>  
> -QGuestAllocator *alloc_init(uint64_t start, uint64_t end)
> +void alloc_init(QGuestAllocator *s, QAllocOpts opts,

As far as I can see, you never use the "opts" parameter in this function
anymore. Should you keep the "s->opts = opts" line below? Or remove this
parameter completely?

> +                uint64_t start, uint64_t end,
> +                size_t page_size)
>  {
> -    QGuestAllocator *s = g_malloc0(sizeof(*s));
>      MemBlock *node;
>  
>      s->start = start;
> @@ -313,26 +301,7 @@ QGuestAllocator *alloc_init(uint64_t start, uint64_t end)
>      node = mlist_new(s->start, s->end - s->start);
>      QTAILQ_INSERT_HEAD(s->free, node, MLIST_ENTNAME);
>  
> -    s->page_size = DEFAULT_PAGE_SIZE;
> -
> -    return s;
> -}
> -
> -QGuestAllocator *alloc_init_flags(QAllocOpts opts,
> -                                  uint64_t start, uint64_t end)
> -{
> -    QGuestAllocator *s = alloc_init(start, end);
> -    s->opts = opts;

i.e. keep this line ^ ?

> -    return s;
> -}
> -
> -void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size)
> -{
> -    /* Can't alter the page_size for an allocator in-use */
> -    g_assert(QTAILQ_EMPTY(allocator->used));
> -
> -    g_assert(is_power_of_2(page_size));
> -    allocator->page_size = page_size;
> +    s->page_size = page_size;
>  }

 Thomas