While mtree_info() handles both ASes and flatviews cases,
the two cases share basically no code. Split mtree_info_flatview()
out of mtree_info() to simplify.

Note: Patch easier to review using 'git-diff --color-moved=blocks'.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 softmmu/memory.c | 72 ++++++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index bfedaf9c4df..3eb6f52de67 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3246,6 +3246,44 @@ static gboolean mtree_info_flatview_free(gpointer key, gpointer value,
     return true;
 }
 
+static void mtree_info_flatview(bool dispatch_tree, bool owner)
+{
+    struct FlatViewInfo fvi = {
+        .counter = 0,
+        .dispatch_tree = dispatch_tree,
+        .owner = owner,
+    };
+    AddressSpace *as;
+    FlatView *view;
+    GArray *fv_address_spaces;
+    GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
+    AccelClass *ac = ACCEL_GET_CLASS(current_accel());
+
+    if (ac->has_memory) {
+        fvi.ac = ac;
+    }
+
+    /* Gather all FVs in one table */
+    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+        view = address_space_get_flatview(as);
+
+        fv_address_spaces = g_hash_table_lookup(views, view);
+        if (!fv_address_spaces) {
+            fv_address_spaces = g_array_new(false, false, sizeof(as));
+            g_hash_table_insert(views, view, fv_address_spaces);
+        }
+
+        g_array_append_val(fv_address_spaces, as);
+    }
+
+    /* Print */
+    g_hash_table_foreach(views, mtree_print_flatview, &fvi);
+
+    /* Free */
+    g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
+    g_hash_table_unref(views);
+}
+
 void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
 {
     MemoryRegionListHead ml_head;
@@ -3253,39 +3291,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
     AddressSpace *as;
 
     if (flatview) {
-        FlatView *view;
-        struct FlatViewInfo fvi = {
-            .counter = 0,
-            .dispatch_tree = dispatch_tree,
-            .owner = owner,
-        };
-        GArray *fv_address_spaces;
-        GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
-        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
-
-        if (ac->has_memory) {
-            fvi.ac = ac;
-        }
-
-        /* Gather all FVs in one table */
-        QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-            view = address_space_get_flatview(as);
-
-            fv_address_spaces = g_hash_table_lookup(views, view);
-            if (!fv_address_spaces) {
-                fv_address_spaces = g_array_new(false, false, sizeof(as));
-                g_hash_table_insert(views, view, fv_address_spaces);
-            }
-
-            g_array_append_val(fv_address_spaces, as);
-        }
-
-        /* Print */
-        g_hash_table_foreach(views, mtree_print_flatview, &fvi);
-
-        /* Free */
-        g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
-        g_hash_table_unref(views);
+        mtree_info_flatview(dispatch_tree, owner);
 
         return;
     }
-- 
2.31.1

On 9/1/21 6:19 PM, Philippe Mathieu-Daudé wrote:
> While mtree_info() handles both ASes and flatviews cases,
> the two cases share basically no code. Split mtree_info_flatview()
> out of mtree_info() to simplify.
> 
> Note: Patch easier to review using 'git-diff --color-moved=blocks'.

Surprisingly git-format-patch choose a better algorithm automatically...

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  softmmu/memory.c | 72 ++++++++++++++++++++++++++----------------------
>  1 file changed, 39 insertions(+), 33 deletions(-)
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index bfedaf9c4df..3eb6f52de67 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -3246,6 +3246,44 @@ static gboolean mtree_info_flatview_free(gpointer key, gpointer value,
>      return true;
>  }
>  
> +static void mtree_info_flatview(bool dispatch_tree, bool owner)
> +{
> +    struct FlatViewInfo fvi = {
> +        .counter = 0,
> +        .dispatch_tree = dispatch_tree,
> +        .owner = owner,
> +    };
> +    AddressSpace *as;
> +    FlatView *view;
> +    GArray *fv_address_spaces;
> +    GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
> +    AccelClass *ac = ACCEL_GET_CLASS(current_accel());
> +
> +    if (ac->has_memory) {
> +        fvi.ac = ac;
> +    }
> +
> +    /* Gather all FVs in one table */
> +    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> +        view = address_space_get_flatview(as);
> +
> +        fv_address_spaces = g_hash_table_lookup(views, view);
> +        if (!fv_address_spaces) {
> +            fv_address_spaces = g_array_new(false, false, sizeof(as));
> +            g_hash_table_insert(views, view, fv_address_spaces);
> +        }
> +
> +        g_array_append_val(fv_address_spaces, as);
> +    }
> +
> +    /* Print */
> +    g_hash_table_foreach(views, mtree_print_flatview, &fvi);
> +
> +    /* Free */
> +    g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
> +    g_hash_table_unref(views);
> +}
> +
>  void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>  {
>      MemoryRegionListHead ml_head;
> @@ -3253,39 +3291,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>      AddressSpace *as;
>  
>      if (flatview) {
> -        FlatView *view;
> -        struct FlatViewInfo fvi = {
> -            .counter = 0,
> -            .dispatch_tree = dispatch_tree,
> -            .owner = owner,
> -        };
> -        GArray *fv_address_spaces;
> -        GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
> -        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
> -
> -        if (ac->has_memory) {
> -            fvi.ac = ac;
> -        }
> -
> -        /* Gather all FVs in one table */
> -        QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> -            view = address_space_get_flatview(as);
> -
> -            fv_address_spaces = g_hash_table_lookup(views, view);
> -            if (!fv_address_spaces) {
> -                fv_address_spaces = g_array_new(false, false, sizeof(as));
> -                g_hash_table_insert(views, view, fv_address_spaces);
> -            }
> -
> -            g_array_append_val(fv_address_spaces, as);
> -        }
> -
> -        /* Print */
> -        g_hash_table_foreach(views, mtree_print_flatview, &fvi);
> -
> -        /* Free */
> -        g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
> -        g_hash_table_unref(views);
> +        mtree_info_flatview(dispatch_tree, owner);
>  
>          return;
>      }
> 

On 01.09.21 18:19, Philippe Mathieu-Daudé wrote:
> While mtree_info() handles both ASes and flatviews cases,
> the two cases share basically no code. Split mtree_info_flatview()
> out of mtree_info() to simplify.
> 
> Note: Patch easier to review using 'git-diff --color-moved=blocks'.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

On 9/1/21 6:19 PM, Philippe Mathieu-Daudé wrote:
> While mtree_info() handles both ASes and flatviews cases,
> the two cases share basically no code. Split mtree_info_flatview()
> out of mtree_info() to simplify.
> 
> Note: Patch easier to review using 'git-diff --color-moved=blocks'.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   softmmu/memory.c | 72 ++++++++++++++++++++++++++----------------------
>   1 file changed, 39 insertions(+), 33 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~