Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]

On 1/23/19 8:20 AM, Alex Bennée wrote:
> 
> Julia Suvorova <jusual@mail.ru> writes:
> 
>> Run qtest with a socket that connects QEMU chardev and test code.
>>
>> Signed-off-by: Julia Suvorova <jusual@mail.ru>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 

>> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
>> +{
>> +    int sock_fd_init;
>> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
>> +    QTestState *qts;
>> +
>> +    g_assert_true(mkdtemp(sock_dir) != NULL);
>> +    sock_path = g_strdup_printf("%s/sock", sock_dir);
>> +
>> +    sock_fd_init = init_socket(sock_path);
>> +
>> +    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
>> +                      sock_path, extra_args);

EWWWW.  Please, let's not bake in this interface.  Relying on shell
splitting is nasty.


>>
>> +/**
>> + * qtest_init_with_serial:
>> + * @extra_args: other arguments to pass to QEMU.  CAUTION: these
>> + * arguments are subject to word splitting and shell evaluation.
>> + * @sock_fd: pointer to store the socket file descriptor for
>> + * connection with serial.
>> + *
>> + * Returns: #QTestState instance.
>> + */
>> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);

I would MUCH rather see an interface that used varargs to pass extra
arguments as a list/array of actual arguments, no shell splitting required.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]