Re: [PATCH 2/2] xen: do not use '%ms' scanf specifier

[Anthony PERARD <anthony@xxxxxxxxxxxxxx>]

On Tue, Jan 07, 2025 at 10:31:40AM +0100, Roger Pau Monne wrote:
> The 'm' parameter used to request auto-allocation of the destination variable
> is not supported on FreeBSD, and as such leads to failures to parse.
> 
> What's more, the current usage of '%ms' with xs_node_scanf() is pointless, as
> it just leads to a double allocation of the same string.  Instead use
> qemu_xen_xs_read() to read the whole xenstore node.
> 
> Fixes: a783f8ad4ec9 ('xen: add a mechanism to automatically create 
> XenDevice-s...')
> Fixes: 9b7737469080 ('hw/xen: update Xen console to XenDevice model')
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  hw/char/xen_console.c | 11 +++++++++--
>  hw/xen/xen-bus.c      |  7 +++++--
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
> index af706c7ef440..18afd214c2f6 100644
> --- a/hw/char/xen_console.c
> +++ b/hw/char/xen_console.c
> @@ -531,6 +531,7 @@ static void xen_console_device_create(XenBackendInstance 
> *backend,
>      const char *name = xen_backend_get_name(backend);
>      unsigned long number;
>      char *fe = NULL, *type = NULL, *output = NULL;
> +    const char *node_path;

Is "const" correct when we are changing to which string `node_path` is
pointing at? Also, why "const"? Also, compiler complains that we can't
free a "const something*".

>      char label[32];
>      XenDevice *xendev = NULL;
>      XenConsole *con;
> @@ -550,7 +551,10 @@ static void xen_console_device_create(XenBackendInstance 
> *backend,
>          goto fail;
>      }
>  
> -    if (xs_node_scanf(xsh, XBT_NULL, fe, "type", errp, "%ms", &type) != 1) {
> +    node_path = g_strdup_printf("%s/type", fe);
> +    type = qemu_xen_xs_read(xsh, XBT_NULL, node_path, NULL);
> +    g_free(node_path);

I feel like we want "xs_node_read()" which would be similair to
xs_node_vscanf() but would simply return the result of
qemu_xen_xs_read(). This would avoid the need format of the node path in
several place in the code. But it's OK like that as well.

Cheers,

-- 
Anthony PERARD