diff mbox series

[v3,12/54] tests/qtest: hd-geo-test: Avoid using hardcoded /tmp

Message ID 20220925113032.1949844-13-bmeng.cn@gmail.com (mailing list archive)
State New, archived
Headers show
Series tests/qtest: Enable running qtest on Windows | expand

Commit Message

Bin Meng Sept. 25, 2022, 11:29 a.m. UTC 
From: Bin Meng <bin.meng@windriver.com>

This case was written to use hardcoded /tmp directory for temporary
files. Update to use g_file_open_tmp() for a portable implementation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v3:
- Split to a separate patch
- Ensure g_autofree variable is initialized
- Use g_steal_pointer() in create_test_img()

 tests/qtest/hd-geo-test.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

Comments

Marc-André Lureau Sept. 26, 2022, 1:11 p.m. UTC | #1 
On Sun, Sep 25, 2022 at 3:58 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> From: Bin Meng <bin.meng@windriver.com>
>
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_file_open_tmp() for a portable implementation.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>



> ---
>
> Changes in v3:
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Use g_steal_pointer() in create_test_img()
>
>  tests/qtest/hd-geo-test.c | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index 413cf964c0..455bc5db5c 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -27,20 +27,19 @@
>
>  static char *create_test_img(int secs)
>  {
> -    char *template = strdup("/tmp/qtest.XXXXXX");
> +    g_autofree char *template = NULL;
>      int fd, ret;
>
> -    fd = mkstemp(template);
> +    fd = g_file_open_tmp("qtest.XXXXXX", &template, NULL);
>      g_assert(fd >= 0);
>      ret = ftruncate(fd, (off_t)secs * 512);
>      close(fd);
>
>      if (ret) {
> -        free(template);
>          template = NULL;
>      }
>
> -    return template;
> +    return g_steal_pointer(&template);
>  }
>
>  typedef struct {
> @@ -422,9 +421,8 @@ static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0,
> 0, 0, 0, 0},
>
>  static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
>  {
> -    const char *template = "/tmp/qtest.XXXXXX";
> -    char *raw_path = strdup(template);
> -    char *qcow2_path = strdup(template);
> +    g_autofree char *raw_path = NULL;
> +    char *qcow2_path;
>      char cmd[100 + 2 * PATH_MAX];
>      uint8_t buf[512] = {};
>      int i, ret, fd, offset;
> @@ -468,7 +466,7 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr,
> uint64_t sectors)
>          offset += 0x10;
>      }
>
> -    fd = mkstemp(raw_path);
> +    fd = g_file_open_tmp("qtest.XXXXXX", &raw_path, NULL);
>      g_assert(fd >= 0);
>      close(fd);
>
> @@ -478,7 +476,7 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr,
> uint64_t sectors)
>      g_assert(ret == sizeof(buf));
>      close(fd);
>
> -    fd = mkstemp(qcow2_path);
> +    fd = g_file_open_tmp("qtest.XXXXXX", &qcow2_path, NULL);
>      g_assert(fd >= 0);
>      close(fd);
>
> @@ -506,7 +504,6 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr,
> uint64_t sectors)
>      free(qemu_img_abs_path);
>
>      unlink(raw_path);
> -    free(raw_path);
>
>      return qcow2_path;
>  }
> @@ -714,7 +711,7 @@ static void test_override(TestArgs *args, CHSResult
> expected[])
>
>      for (i = 0; i < args->n_drives; i++) {
>          unlink(args->drives[i]);
> -        free(args->drives[i]);
> +        g_free(args->drives[i]);
>      }
>      g_free(args->drives);
>      g_strfreev(args->argv);
> @@ -867,7 +864,7 @@ static void test_override_scsi_hot_unplug(void)
>
>      for (i = 0; i < args->n_drives; i++) {
>          unlink(args->drives[i]);
> -        free(args->drives[i]);
> +        g_free(args->drives[i]);
>      }
>      g_free(args->drives);
>      g_strfreev(args->argv);
> @@ -927,7 +924,7 @@ static void test_override_virtio_hot_unplug(void)
>
>      for (i = 0; i < args->n_drives; i++) {
>          unlink(args->drives[i]);
> -        free(args->drives[i]);
> +        g_free(args->drives[i]);
>      }
>      g_free(args->drives);
>      g_strfreev(args->argv);
> @@ -987,7 +984,7 @@ test_add_done:
>      for (i = 0; i < backend_last; i++) {
>          if (img_file_name[i]) {
>              unlink(img_file_name[i]);
> -            free(img_file_name[i]);
> +            g_free(img_file_name[i]);
>          }
>      }
>
> --
> 2.34.1
>
>
>
Thomas Huth Sept. 26, 2022, 3:38 p.m. UTC | #2 
On 25/09/2022 13.29, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> This case was written to use hardcoded /tmp directory for temporary
> files. Update to use g_file_open_tmp() for a portable implementation.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
> Changes in v3:
> - Split to a separate patch
> - Ensure g_autofree variable is initialized
> - Use g_steal_pointer() in create_test_img()
> 
>   tests/qtest/hd-geo-test.c | 25 +++++++++++--------------
>   1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index 413cf964c0..455bc5db5c 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -27,20 +27,19 @@
>   
>   static char *create_test_img(int secs)
>   {
> -    char *template = strdup("/tmp/qtest.XXXXXX");
> +    g_autofree char *template = NULL;
>       int fd, ret;
>   
> -    fd = mkstemp(template);
> +    fd = g_file_open_tmp("qtest.XXXXXX", &template, NULL);
>       g_assert(fd >= 0);
>       ret = ftruncate(fd, (off_t)secs * 512);
>       close(fd);
>   
>       if (ret) {
> -        free(template);
>           template = NULL;
>       }
>   
> -    return template;
> +    return g_steal_pointer(&template);

Just a matter of taste, but in this case, I'd rather prefer to not use 
g_autofree + g_steal_pointer and simply replace the "free" with a "g_free" 
instead.

  Thomas
diff mbox series

Patch

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index 413cf964c0..455bc5db5c 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -27,20 +27,19 @@ 
 
 static char *create_test_img(int secs)
 {
-    char *template = strdup("/tmp/qtest.XXXXXX");
+    g_autofree char *template = NULL;
     int fd, ret;
 
-    fd = mkstemp(template);
+    fd = g_file_open_tmp("qtest.XXXXXX", &template, NULL);
     g_assert(fd >= 0);
     ret = ftruncate(fd, (off_t)secs * 512);
     close(fd);
 
     if (ret) {
-        free(template);
         template = NULL;
     }
 
-    return template;
+    return g_steal_pointer(&template);
 }
 
 typedef struct {
@@ -422,9 +421,8 @@  static MBRpartitions empty_mbr = { {false, 0, 0, 0, 0, 0, 0, 0, 0},
 
 static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
 {
-    const char *template = "/tmp/qtest.XXXXXX";
-    char *raw_path = strdup(template);
-    char *qcow2_path = strdup(template);
+    g_autofree char *raw_path = NULL;
+    char *qcow2_path;
     char cmd[100 + 2 * PATH_MAX];
     uint8_t buf[512] = {};
     int i, ret, fd, offset;
@@ -468,7 +466,7 @@  static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
         offset += 0x10;
     }
 
-    fd = mkstemp(raw_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &raw_path, NULL);
     g_assert(fd >= 0);
     close(fd);
 
@@ -478,7 +476,7 @@  static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
     g_assert(ret == sizeof(buf));
     close(fd);
 
-    fd = mkstemp(qcow2_path);
+    fd = g_file_open_tmp("qtest.XXXXXX", &qcow2_path, NULL);
     g_assert(fd >= 0);
     close(fd);
 
@@ -506,7 +504,6 @@  static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
     free(qemu_img_abs_path);
 
     unlink(raw_path);
-    free(raw_path);
 
     return qcow2_path;
 }
@@ -714,7 +711,7 @@  static void test_override(TestArgs *args, CHSResult expected[])
 
     for (i = 0; i < args->n_drives; i++) {
         unlink(args->drives[i]);
-        free(args->drives[i]);
+        g_free(args->drives[i]);
     }
     g_free(args->drives);
     g_strfreev(args->argv);
@@ -867,7 +864,7 @@  static void test_override_scsi_hot_unplug(void)
 
     for (i = 0; i < args->n_drives; i++) {
         unlink(args->drives[i]);
-        free(args->drives[i]);
+        g_free(args->drives[i]);
     }
     g_free(args->drives);
     g_strfreev(args->argv);
@@ -927,7 +924,7 @@  static void test_override_virtio_hot_unplug(void)
 
     for (i = 0; i < args->n_drives; i++) {
         unlink(args->drives[i]);
-        free(args->drives[i]);
+        g_free(args->drives[i]);
     }
     g_free(args->drives);
     g_strfreev(args->argv);
@@ -987,7 +984,7 @@  test_add_done:
     for (i = 0; i < backend_last; i++) {
         if (img_file_name[i]) {
             unlink(img_file_name[i]);
-            free(img_file_name[i]);
+            g_free(img_file_name[i]);
         }
     }