Re: [PATCH v2 1/3] fuzz: enable dynamic args for generic-fuzz configs

From: Thomas Huth <thuth@redhat.com>
To: Alexander Bulekov <alxndr@bu.edu>, qemu-devel@nongnu.org
Cc: Laurent Vivier <lvivier@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>, Bandan Das <bsd@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [PATCH v2 1/3] fuzz: enable dynamic args for generic-fuzz configs
Date: Mon, 18 Jan 2021 10:25:06 +0100	[thread overview]
Message-ID: <076dae16-6e1e-dab8-b822-926f1b4a9a76@redhat.com> (raw)
In-Reply-To: <20210117230924.449676-2-alxndr@bu.edu>

On 18/01/2021 00.09, Alexander Bulekov wrote:
> For some device configurations, it is useful to configure some
> resources, and adjust QEMU arguments at runtime, prior to fuzzing. This
> patch adds an "argfunc" to generic the generic_fuzz_config. When
> specified, it is responsible for configuring the resources and returning
> a string containing the corresponding QEMU arguments. This can be useful
> for targets that rely on e.g.:
>   * a temporary qcow2 image
>   * a temporary directory
>   * an unused TCP port used to bind the VNC server
> 
> Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
> ---
>   tests/qtest/fuzz/generic_fuzz.c         | 10 +++++++++-
>   tests/qtest/fuzz/generic_fuzz_configs.h |  1 +
>   2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c
> index be76d47d2d..6adf62a5be 100644
> --- a/tests/qtest/fuzz/generic_fuzz.c
> +++ b/tests/qtest/fuzz/generic_fuzz.c
> @@ -936,12 +936,20 @@ static GString *generic_fuzz_cmdline(FuzzTarget *t)
>   
>   static GString *generic_fuzz_predefined_config_cmdline(FuzzTarget *t)
>   {
> +    gchar *args;
>       const generic_fuzz_config *config;
>       g_assert(t->opaque);
>   
>       config = t->opaque;
>       setenv("QEMU_AVOID_DOUBLE_FETCH", "1", 1);
> -    setenv("QEMU_FUZZ_ARGS", config->args, 1);
> +    if (config->argfunc) {
> +        args = config->argfunc();
> +        setenv("QEMU_FUZZ_ARGS", args, 1);
> +        g_free(args);
> +    } else {
> +        g_assert_nonnull(config->args);
> +        setenv("QEMU_FUZZ_ARGS", config->args, 1);
> +    }
>       setenv("QEMU_FUZZ_OBJECTS", config->objects, 1);
>       return generic_fuzz_cmdline(t);
>   }
> diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h
> index 7fed035345..1a133655ee 100644
> --- a/tests/qtest/fuzz/generic_fuzz_configs.h
> +++ b/tests/qtest/fuzz/generic_fuzz_configs.h
> @@ -16,6 +16,7 @@
>   
>   typedef struct generic_fuzz_config {
>       const char *name, *args, *objects;
> +    gchar* (*argfunc)(void); /* Result must be free
Reviewed-by: Thomas Huth <thuth@redhat.com>

... would it make sense to also add a cleanup function pointer here, so that 
the resources can also be freed cleanly after a test has succeeded (instead 
of using atexit() like suggested in your third patch)? Well, just an idea, 
it still can be done in a later patch.