[v1] fw_cfg: Run tests on big-endian

[PATCH 3/7] tests/libqos/fw_cfg: Document pc_fw_cfg_init to drop pc_fw_cfg_uninit

Posted by Philippe Mathieu-Daudé 6 years, 4 months ago

Document pc_fw_cfg_init() return value must be released
with g_free(). Directly calling g_free() we don't really
need pc_fw_cfg_uninit(): remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tests/fw_cfg-test.c      | 22 +++++++++++-----------
 tests/libqos/fw_cfg.h    | 14 +++++++++-----
 tests/libqos/malloc-pc.c |  2 +-
 3 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
index 1d3147f821..53ae82f7c8 100644
--- a/tests/fw_cfg-test.c
+++ b/tests/fw_cfg-test.c
@@ -36,7 +36,7 @@ static void test_fw_cfg_signature(void)
     buf[4] = 0;
 
     g_assert_cmpstr(buf, ==, "QEMU");
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -52,7 +52,7 @@ static void test_fw_cfg_id(void)
     id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
     g_assert((id == 1) ||
              (id == 3));
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -73,7 +73,7 @@ static void test_fw_cfg_uuid(void)
     qfw_cfg_get(fw_cfg, FW_CFG_UUID, buf, 16);
     g_assert(memcmp(buf, uuid, sizeof(buf)) == 0);
 
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 
 }
@@ -88,7 +88,7 @@ static void test_fw_cfg_ram_size(void)
 
     g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE), ==, ram_size);
 
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -102,7 +102,7 @@ static void test_fw_cfg_nographic(void)
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
 
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -116,7 +116,7 @@ static void test_fw_cfg_nb_cpus(void)
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NB_CPUS), ==, nb_cpus);
 
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -129,7 +129,7 @@ static void test_fw_cfg_max_cpus(void)
     fw_cfg = pc_fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_MAX_CPUS), ==, max_cpus);
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -158,7 +158,7 @@ static void test_fw_cfg_numa(void)
 
     g_free(node_mask);
     g_free(cpu_mask);
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -171,7 +171,7 @@ static void test_fw_cfg_boot_menu(void)
     fw_cfg = pc_fw_cfg_init(s);
 
     g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -190,7 +190,7 @@ static void test_fw_cfg_reboot_timeout(void)
     g_assert_cmpint(filesize, ==, sizeof(reboot_timeout));
     reboot_timeout = le32_to_cpu(reboot_timeout);
     g_assert_cmpint(reboot_timeout, ==, 15);
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
@@ -209,7 +209,7 @@ static void test_fw_cfg_splash_time(void)
     g_assert_cmpint(filesize, ==, sizeof(splash_time));
     splash_time = le16_to_cpu(splash_time);
     g_assert_cmpint(splash_time, ==, 12);
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
     qtest_quit(s);
 }
 
diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
index 3247fd4000..6316f4c354 100644
--- a/tests/libqos/fw_cfg.h
+++ b/tests/libqos/fw_cfg.h
@@ -54,14 +54,18 @@ QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
  */
 QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
 
+/**
+ * pc_fw_cfg_init():
+ * @qts: The #QTestState that will be referred to by the QFWCFG object.
+ *
+ * This function is specific to the PC machine (X86).
+ *
+ * Returns a newly allocated QFWCFG object which must be released
+ * with a call to g_free() when no longer required.
+ */
 static inline QFWCFG *pc_fw_cfg_init(QTestState *qts)
 {
     return io_fw_cfg_init(qts, 0x510);
 }
 
-static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
-{
-    g_free(fw_cfg);
-}
-
 #endif
diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
index 6f92ce4135..949a99361d 100644
--- a/tests/libqos/malloc-pc.c
+++ b/tests/libqos/malloc-pc.c
@@ -29,5 +29,5 @@ void pc_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags)
     alloc_init(s, flags, 1 << 20, MIN(ram_size, 0xE0000000), PAGE_SIZE);
 
     /* clean-up */
-    pc_fw_cfg_uninit(fw_cfg);
+    g_free(fw_cfg);
 }
-- 
2.20.1

Re: [PATCH 3/7] tests/libqos/fw_cfg: Document pc_fw_cfg_init to drop pc_fw_cfg_uninit

Posted by Laszlo Ersek 6 years, 4 months ago

On 10/04/19 00:54, Philippe Mathieu-Daudé wrote:
> Document pc_fw_cfg_init() return value must be released
> with g_free(). Directly calling g_free() we don't really
> need pc_fw_cfg_uninit(): remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tests/fw_cfg-test.c      | 22 +++++++++++-----------
>  tests/libqos/fw_cfg.h    | 14 +++++++++-----
>  tests/libqos/malloc-pc.c |  2 +-
>  3 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/fw_cfg-test.c b/tests/fw_cfg-test.c
> index 1d3147f821..53ae82f7c8 100644
> --- a/tests/fw_cfg-test.c
> +++ b/tests/fw_cfg-test.c
> @@ -36,7 +36,7 @@ static void test_fw_cfg_signature(void)
>      buf[4] = 0;
>  
>      g_assert_cmpstr(buf, ==, "QEMU");
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -52,7 +52,7 @@ static void test_fw_cfg_id(void)
>      id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
>      g_assert((id == 1) ||
>               (id == 3));
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -73,7 +73,7 @@ static void test_fw_cfg_uuid(void)
>      qfw_cfg_get(fw_cfg, FW_CFG_UUID, buf, 16);
>      g_assert(memcmp(buf, uuid, sizeof(buf)) == 0);
>  
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  
>  }
> @@ -88,7 +88,7 @@ static void test_fw_cfg_ram_size(void)
>  
>      g_assert_cmpint(qfw_cfg_get_u64(fw_cfg, FW_CFG_RAM_SIZE), ==, ram_size);
>  
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -102,7 +102,7 @@ static void test_fw_cfg_nographic(void)
>  
>      g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NOGRAPHIC), ==, 0);
>  
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -116,7 +116,7 @@ static void test_fw_cfg_nb_cpus(void)
>  
>      g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_NB_CPUS), ==, nb_cpus);
>  
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -129,7 +129,7 @@ static void test_fw_cfg_max_cpus(void)
>      fw_cfg = pc_fw_cfg_init(s);
>  
>      g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_MAX_CPUS), ==, max_cpus);
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -158,7 +158,7 @@ static void test_fw_cfg_numa(void)
>  
>      g_free(node_mask);
>      g_free(cpu_mask);
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -171,7 +171,7 @@ static void test_fw_cfg_boot_menu(void)
>      fw_cfg = pc_fw_cfg_init(s);
>  
>      g_assert_cmpint(qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_MENU), ==, boot_menu);
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -190,7 +190,7 @@ static void test_fw_cfg_reboot_timeout(void)
>      g_assert_cmpint(filesize, ==, sizeof(reboot_timeout));
>      reboot_timeout = le32_to_cpu(reboot_timeout);
>      g_assert_cmpint(reboot_timeout, ==, 15);
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> @@ -209,7 +209,7 @@ static void test_fw_cfg_splash_time(void)
>      g_assert_cmpint(filesize, ==, sizeof(splash_time));
>      splash_time = le16_to_cpu(splash_time);
>      g_assert_cmpint(splash_time, ==, 12);
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>      qtest_quit(s);
>  }
>  
> diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
> index 3247fd4000..6316f4c354 100644
> --- a/tests/libqos/fw_cfg.h
> +++ b/tests/libqos/fw_cfg.h
> @@ -54,14 +54,18 @@ QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
>   */
>  QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
>  
> +/**
> + * pc_fw_cfg_init():
> + * @qts: The #QTestState that will be referred to by the QFWCFG object.
> + *
> + * This function is specific to the PC machine (X86).
> + *
> + * Returns a newly allocated QFWCFG object which must be released
> + * with a call to g_free() when no longer required.
> + */
>  static inline QFWCFG *pc_fw_cfg_init(QTestState *qts)
>  {
>      return io_fw_cfg_init(qts, 0x510);
>  }
>  
> -static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
> -{
> -    g_free(fw_cfg);
> -}
> -
>  #endif
> diff --git a/tests/libqos/malloc-pc.c b/tests/libqos/malloc-pc.c
> index 6f92ce4135..949a99361d 100644
> --- a/tests/libqos/malloc-pc.c
> +++ b/tests/libqos/malloc-pc.c
> @@ -29,5 +29,5 @@ void pc_alloc_init(QGuestAllocator *s, QTestState *qts, QAllocOpts flags)
>      alloc_init(s, flags, 1 << 20, MIN(ram_size, 0xE0000000), PAGE_SIZE);
>  
>      /* clean-up */
> -    pc_fw_cfg_uninit(fw_cfg);
> +    g_free(fw_cfg);
>  }
> 

I'm getting the impression we're trying to remove "PC" (x86) references
from the cleanup action :)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

[PATCH 1/7] tests/libqos/fw_cfg: Document io_fw_cfg_init to drop io_fw_cfg_uninit
[PATCH 2/7] tests/libqos/fw_cfg: Document mm_fw_cfg_init to drop mm_fw_cfg_uninit
[PATCH 3/7] tests/libqos/fw_cfg: Document pc_fw_cfg_init to drop pc_fw_cfg_uninit
[PATCH 4/7] tests/fw_cfg: Let the tests use a context
[PATCH 5/7] tests/libqos/fw_cfg: Pass QTestState as argument
[PATCH 6/7] tests/fw_cfg: Declare one QFWCFG for all tests
[PATCH 7/7] tests/fw_cfg: Run the tests on big-endian targets