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

[Thomas Huth]

On 24/08/2022 11.39, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
>
> Use g_get_tmp_dir() to get the directory to use for temporary files.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>   tests/qtest/fuzz/generic_fuzz_configs.h |  6 ++++--
>   tests/qtest/ahci-test.c                 | 15 +++++++++++----
>   tests/qtest/aspeed_smc-test.c           |  4 +++-
>   tests/qtest/boot-serial-test.c          |  8 ++++++--
>   tests/qtest/cxl-test.c                  |  9 ++++++---
>   tests/qtest/fdc-test.c                  |  4 +++-
>   tests/qtest/fuzz/virtio_blk_fuzz.c      |  2 +-
>   tests/qtest/hd-geo-test.c               |  8 ++++----
>   tests/qtest/ide-test.c                  |  8 ++++++--
>   tests/qtest/libqtest.c                  | 10 +++++++---
>   tests/qtest/migration-test.c            |  4 +++-
>   tests/qtest/pflash-cfi02-test.c         |  7 +++++--
>   tests/qtest/qmp-test.c                  |  4 +++-
>   tests/qtest/vhost-user-blk-test.c       |  3 ++-
>   tests/qtest/vhost-user-test.c           |  3 ++-
>   tests/qtest/virtio-blk-test.c           |  2 +-
>   tests/qtest/virtio-scsi-test.c          |  3 ++-
>   tests/unit/test-image-locking.c         |  6 ++++--
>   tests/unit/test-qga.c                   |  2 +-
>   tests/vhost-user-bridge.c               |  3 ++-
>   20 files changed, 76 insertions(+), 35 deletions(-)
>
> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
> b/tests/qtest/fuzz/generic_fuzz_configs.h
> index 0775e6702b..d0f9961187 100644
> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> @@ -20,13 +20,15 @@ typedef struct generic_fuzz_config {
>   } generic_fuzz_config;
>   
>   static inline gchar *generic_fuzzer_virtio_9p_args(void){
> -    char tmpdir[] = "/tmp/qemu-fuzz.XXXXXX";
> +    char *tmpdir = g_strdup_printf("%s/qemu-fuzz.XXXXXX", g_get_tmp_dir());
>       g_assert_nonnull(g_mkdtemp(tmpdir));

Could you switch this to g_dir_make_tmp(), please ? (see 
https://docs.gtk.org/glib/type_func.Dir.make_tmp.html )

(and as David said, please use g_autofree to avoid the g_free() later)

> -    return g_strdup_printf("-machine q35 -nodefaults "
> +    gchar *args = g_strdup_printf("-machine q35 -nodefaults "
>       "-device virtio-9p,fsdev=hshare,mount_tag=hshare "
>       "-fsdev local,id=hshare,path=%s,security_model=mapped-xattr,"
>       "writeout=immediate,fmode=0600,dmode=0700", tmpdir);
> +    g_free(tmpdir);
> +    return args;
>   }
>   
>   const generic_fuzz_config predefined_configs[] = {
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index f1e510b0ac..f26cd6f86f 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.XXXXXX";
> -static char debug_path[] = "/tmp/qtest-blkdebug.XXXXXX";
> -static char mig_socket[] = "/tmp/qtest-migration.XXXXXX";
> +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,7 +1437,7 @@ static void test_ncq_simple(void)
>   
>   static int prepare_iso(size_t size, unsigned char **buf, char **name)
>   {
> -    char cdrom_path[] = "/tmp/qtest.iso.XXXXXX";
> +    char *cdrom_path = g_strdup_printf("%s/qtest.iso.XXXXXX", g_get_tmp_dir());
>       unsigned char *patt;
>       ssize_t ret;
>       int fd = mkstemp(cdrom_path);

Would it be better to use g_file_open_tmp() here?
(see https://docs.gtk.org/glib/func.file_open_tmp.html)

> @@ -1872,6 +1873,7 @@ int main(int argc, char **argv)
>       }
>   
>       /* Create a temporary image */
> +    tmp_path = g_strdup_printf("%s/qtest.XXXXXX", g_get_tmp_dir());
>       fd = mkstemp(tmp_path);
>       g_assert(fd >= 0);

g_file_open_tmp() ?

Also for the other temporary files in this test?

> diff --git a/tests/qtest/aspeed_smc-test.c b/tests/qtest/aspeed_smc-test.c
> index 05ce941566..cab769459c 100644
> --- a/tests/qtest/aspeed_smc-test.c
> +++ b/tests/qtest/aspeed_smc-test.c
> @@ -608,7 +608,7 @@ static void test_write_block_protect_bottom_bit(void)
>       flash_reset();
>   }
>   
> -static char tmp_path[] = "/tmp/qtest.m25p80.XXXXXX";
> +static char *tmp_path;
>   
>   int main(int argc, char **argv)
>   {
> @@ -617,6 +617,7 @@ int main(int argc, char **argv)
>   
>       g_test_init(&argc, &argv, NULL);
>   
> +    tmp_path = g_strdup_printf("%s/qtest.m25p80.XXXXXX", g_get_tmp_dir());
>       fd = mkstemp(tmp_path);

g_file_open_tmp() ?

> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> index 2f99d71cab..404adcfa20 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -224,8 +224,10 @@ static bool check_guest_output(QTestState *qts, const 
> testdef_t *test, int fd)
>   static void test_machine(const void *data)
>   {
>       const testdef_t *test = data;
> -    char serialtmp[] = "/tmp/qtest-boot-serial-sXXXXXX";
> -    char codetmp[] = "/tmp/qtest-boot-serial-cXXXXXX";
> +    char *serialtmp = g_strdup_printf("%s/qtest-boot-serial-sXXXXXX",
> +                                      g_get_tmp_dir());
> +    char *codetmp = g_strdup_printf("%s/qtest-boot-serial-cXXXXXX",
> +                                    g_get_tmp_dir());

These look like good candidates for g_file_open_tmp(), too.

Please check also the other spots in this patch, I haven't looked through 
all of them now...

 Thomas