Re: [PATCH v2 02/11] tests/qtest/boot-serial-test: Simplify test_machine() a bit

[Thomas Huth]

On 19/01/2023 15.58, Philippe Mathieu-Daudé wrote:
> Slighly modify test_machine() to simplify next commit review.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/boot-serial-test.c | 21 +++++++--------------
>   1 file changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
> index 3aef3a97a9..3a854b0174 100644
> --- a/tests/qtest/boot-serial-test.c
> +++ b/tests/qtest/boot-serial-test.c
> @@ -227,7 +227,6 @@ static void test_machine(const void *data)
>       g_autofree char *serialtmp = NULL;
>       g_autofree char *codetmp = NULL;
>       const char *codeparam = "";
> -    const uint8_t *code = NULL;
>       QTestState *qts;
>       int ser_fd;
>   
> @@ -235,21 +234,13 @@ static void test_machine(const void *data)
>       g_assert(ser_fd != -1);
>       close(ser_fd);
>   
> -    if (test->kernel) {
> -        code = test->kernel;
> -        codeparam = "-kernel";
> -    } else if (test->bios) {
> -        code = test->bios;
> -        codeparam = "-bios";
> -    }
> -
> -    if (code) {
> +    if (test->kernel || test->bios) {
>           ssize_t wlen;
>           int code_fd;
>   
>           code_fd = g_file_open_tmp("qtest-boot-serial-cXXXXXX", &codetmp, NULL);
>           g_assert(code_fd != -1);
> -        wlen = write(code_fd, code, test->codesize);
> +        wlen = write(code_fd, test->kernel ? : test->bios, test->codesize);

Just a matter of taste, but I prefer the Elvis operator without space in 
between.

>           g_assert(wlen == test->codesize);
>           close(code_fd);
>       }
> @@ -258,12 +249,14 @@ static void test_machine(const void *data)
>        * Make sure that this test uses tcg if available: It is used as a
>        * fast-enough smoketest for that.
>        */
> -    qts = qtest_initf("%s %s -M %s -no-shutdown "
> +    qts = qtest_initf("%s %s %s -M %s -no-shutdown "
>                         "-chardev file,id=serial0,path=%s "
>                         "-serial chardev:serial0 -accel tcg -accel kvm %s",
> -                      codeparam, code ? codetmp : "", test->machine,
> +                      codeparam,

You removed the initialization of codeparam, so this is now always the empty 
string, isn't it? ... please just remove it completely now.

> +                      test->kernel ? "-kernel " : test->bios ? "-bios " : "",
> +                      codetmp ? : "", test->machine,
>                         serialtmp, test->extra);
> -    if (code) {
> +    if (codetmp) {
>           unlink(codetmp);
>       }
>   

 Thomas