Re: [PATCH v2 07/39] tests: Avoid using hardcoded /tmp in test cases

2022-09-22 Thread Markus Armbruster
Marc-André Lureau  writes:

> Hi
>
> On Tue, Sep 20, 2022 at 2:47 PM Bin Meng  wrote:
>
>> From: Bin Meng 
>>
>> Lots of test cases were written to use hardcoded /tmp directory for
>> temporary files. To avoid this, we update them to use g_dir_make_tmp()
>> or g_file_open_tmp() when appropriate.
>>
>> Signed-off-by: Bin Meng 
>> ---
>>
>> Changes in v2:
>> - Use g_dir_make_tmp(), g_file_open_tmp() when appropriate
>>
>>  tests/qtest/fuzz/generic_fuzz_configs.h |  4 ++--
>>  tests/qtest/ahci-test.c | 19 +++
>>  tests/qtest/aspeed_smc-test.c   |  5 ++---
>>  tests/qtest/boot-serial-test.c  |  9 +
>>  tests/qtest/cxl-test.c  | 15 ++-
>>  tests/qtest/fdc-test.c  |  5 +++--
>>  tests/qtest/fuzz/virtio_blk_fuzz.c  |  4 ++--
>>  tests/qtest/hd-geo-test.c   | 24 +++-
>>  tests/qtest/ide-test.c  | 10 ++
>>  tests/qtest/libqtest.c  | 12 
>>  tests/qtest/migration-test.c|  7 ---
>>  tests/qtest/pflash-cfi02-test.c |  8 +---
>>  tests/qtest/qmp-test.c  |  6 --
>>  tests/qtest/vhost-user-blk-test.c   |  3 ++-
>>  tests/qtest/vhost-user-test.c   |  8 
>>  tests/qtest/virtio-blk-test.c   |  4 ++--
>>  tests/qtest/virtio-scsi-test.c  |  4 ++--
>>  tests/unit/test-image-locking.c |  8 
>>  tests/unit/test-qga.c   |  2 +-
>>  tests/vhost-user-bridge.c   |  3 +--
>>  20 files changed, 85 insertions(+), 75 deletions(-)
>>
>> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h
>> b/tests/qtest/fuzz/generic_fuzz_configs.h
>> index 0775e6702b..a825b78c14 100644
>> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
>> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
>> @@ -20,8 +20,8 @@ typedef struct generic_fuzz_config {
>>  } generic_fuzz_config;
>>
>>  static inline gchar *generic_fuzzer_virtio_9p_args(void){
>> -char tmpdir[] = "/tmp/qemu-fuzz.XX";
>> -g_assert_nonnull(g_mkdtemp(tmpdir));
>> +g_autofree char *tmpdir = g_dir_make_tmp("qemu-fuzz.XX", NULL);
>> +g_assert_nonnull(tmpdir);
>>
>>  return g_strdup_printf("-machine q35 -nodefaults "
>>  "-device virtio-9p,fsdev=hshare,mount_tag=hshare "
>> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
>> index f1e510b0ac..00524f14c6 100644
>> --- a/tests/qtest/ahci-test.c
>> +++ b/tests/qtest/ahci-test.c
>> @@ -44,9 +44,9 @@
>>  #define TEST_IMAGE_SIZE_MB_SMALL 64
>>
>>  /*** Globals ***/
>> -static char tmp_path[] = "/tmp/qtest.XX";
>> -static char debug_path[] = "/tmp/qtest-blkdebug.XX";
>> -static char mig_socket[] = "/tmp/qtest-migration.XX";
>> +static char *tmp_path;
>> +static char *debug_path;
>> +static char *mig_socket;
>>  static bool ahci_pedantic;
>>  static const char *imgfmt;
>>  static unsigned test_image_size_mb;
>> @@ -1437,10 +1437,10 @@ static void test_ncq_simple(void)
>>
>>  static int prepare_iso(size_t size, unsigned char **buf, char **name)
>>  {
>> -char cdrom_path[] = "/tmp/qtest.iso.XX";
>> +g_autofree char *cdrom_path;
>>
>
> Whenever introducing g_auto* usage, please make sure to initialize the
> variable to NULL or a correct value.

Potential food for checkpatch.pl.

[...]




Re: [PATCH v2 07/39] tests: Avoid using hardcoded /tmp in test cases

2022-09-22 Thread Marc-André Lureau
Hi

On Tue, Sep 20, 2022 at 2:47 PM Bin Meng  wrote:

> From: Bin Meng 
>
> Lots of test cases were written to use hardcoded /tmp directory for
> temporary files. To avoid this, we update them to use g_dir_make_tmp()
> or g_file_open_tmp() when appropriate.
>
> Signed-off-by: Bin Meng 
> ---
>
> Changes in v2:
> - Use g_dir_make_tmp(), g_file_open_tmp() when appropriate
>
>  tests/qtest/fuzz/generic_fuzz_configs.h |  4 ++--
>  tests/qtest/ahci-test.c | 19 +++
>  tests/qtest/aspeed_smc-test.c   |  5 ++---
>  tests/qtest/boot-serial-test.c  |  9 +
>  tests/qtest/cxl-test.c  | 15 ++-
>  tests/qtest/fdc-test.c  |  5 +++--
>  tests/qtest/fuzz/virtio_blk_fuzz.c  |  4 ++--
>  tests/qtest/hd-geo-test.c   | 24 +++-
>  tests/qtest/ide-test.c  | 10 ++
>  tests/qtest/libqtest.c  | 12 
>  tests/qtest/migration-test.c|  7 ---
>  tests/qtest/pflash-cfi02-test.c |  8 +---
>  tests/qtest/qmp-test.c  |  6 --
>  tests/qtest/vhost-user-blk-test.c   |  3 ++-
>  tests/qtest/vhost-user-test.c   |  8 
>  tests/qtest/virtio-blk-test.c   |  4 ++--
>  tests/qtest/virtio-scsi-test.c  |  4 ++--
>  tests/unit/test-image-locking.c |  8 
>  tests/unit/test-qga.c   |  2 +-
>  tests/vhost-user-bridge.c   |  3 +--
>  20 files changed, 85 insertions(+), 75 deletions(-)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h
> b/tests/qtest/fuzz/generic_fuzz_configs.h
> index 0775e6702b..a825b78c14 100644
> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> @@ -20,8 +20,8 @@ typedef struct generic_fuzz_config {
>  } generic_fuzz_config;
>
>  static inline gchar *generic_fuzzer_virtio_9p_args(void){
> -char tmpdir[] = "/tmp/qemu-fuzz.XX";
> -g_assert_nonnull(g_mkdtemp(tmpdir));
> +g_autofree char *tmpdir = g_dir_make_tmp("qemu-fuzz.XX", NULL);
> +g_assert_nonnull(tmpdir);
>
>  return g_strdup_printf("-machine q35 -nodefaults "
>  "-device virtio-9p,fsdev=hshare,mount_tag=hshare "
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index f1e510b0ac..00524f14c6 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -44,9 +44,9 @@
>  #define TEST_IMAGE_SIZE_MB_SMALL 64
>
>  /*** Globals ***/
> -static char tmp_path[] = "/tmp/qtest.XX";
> -static char debug_path[] = "/tmp/qtest-blkdebug.XX";
> -static char mig_socket[] = "/tmp/qtest-migration.XX";
> +static char *tmp_path;
> +static char *debug_path;
> +static char *mig_socket;
>  static bool ahci_pedantic;
>  static const char *imgfmt;
>  static unsigned test_image_size_mb;
> @@ -1437,10 +1437,10 @@ static void test_ncq_simple(void)
>
>  static int prepare_iso(size_t size, unsigned char **buf, char **name)
>  {
> -char cdrom_path[] = "/tmp/qtest.iso.XX";
> +g_autofree char *cdrom_path;
>

Whenever introducing g_auto* usage, please make sure to initialize the
variable to NULL or a correct value.

 unsigned char *patt;
>  ssize_t ret;
> -int fd = mkstemp(cdrom_path);
> +int fd = g_file_open_tmp("qtest.iso.XX", &cdrom_path, NULL);
>
>  g_assert(fd != -1);
>  g_assert(buf);
> @@ -1872,7 +1872,7 @@ int main(int argc, char **argv)
>  }
>
>  /* Create a temporary image */
> -fd = mkstemp(tmp_path);
> +fd = g_file_open_tmp("qtest.XX", &tmp_path, NULL);
>  g_assert(fd >= 0);
>  if (have_qemu_img()) {
>  imgfmt = "qcow2";
> @@ -1889,12 +1889,12 @@ int main(int argc, char **argv)
>  close(fd);
>
>  /* Create temporary blkdebug instructions */
> -fd = mkstemp(debug_path);
> +fd = g_file_open_tmp("qtest-blkdebug.XX", &debug_path, NULL);
>  g_assert(fd >= 0);
>  close(fd);
>
>  /* Reserve a hollow file to use as a socket for migration tests */
> -fd = mkstemp(mig_socket);
> +fd = g_file_open_tmp("qtest-migration.XX", &mig_socket, NULL);
>  g_assert(fd >= 0);
>  close(fd);
>
> @@ -1947,8 +1947,11 @@ int main(int argc, char **argv)
>
>  /* Cleanup */
>  unlink(tmp_path);
> +g_free(tmp_path);
>  unlink(debug_path);
> +g_free(debug_path);
>  unlink(mig_socket);
> +g_free(mig_socket);
>
>  return ret;
>  }
> diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
> index 05ce941566..5e16b5c9a5 100644
> --- a/tests/qtest/aspeed_smc-test.c
> +++ b/tests/qtest/aspeed_smc-test.c
> @@ -608,16 +608,15 @@ static void test_write_block_protect_bottom_bit(void)
>  flash_reset();
>  }
>
> -static char tmp_path[] = "/tmp/qtest.m25p80.XX";
> -
>  int main(int argc, char **argv)
>  {
> +g_autofree char *tmp_path;
>  int ret;
>  int fd;
>
>  g_test_init(&argc, &argv, NULL);
>
> -fd = mkstemp(t