[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/2] xen: do not use '%ms' scanf specifier
From: |
David Woodhouse |
Subject: |
Re: [PATCH 2/2] xen: do not use '%ms' scanf specifier |
Date: |
Fri, 10 Jan 2025 08:08:11 +0000 |
User-agent: |
Evolution 3.52.3-0ubuntu1 |
On Thu, 2025-01-09 at 17:55 +0100, Roger Pau Monné wrote:
> On Thu, Jan 09, 2025 at 11:25:13AM +0000, David Woodhouse wrote:
> > On Thu, 2025-01-09 at 11:59 +0100, Anthony PERARD wrote:
> > >
> > > > 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.
> >
> > If you look at the other callers of qemu_xen_xs_read(), it looks like
> > the majority of them create the path with snprintf and then pass it in.
> > Or with g_strdup_printf(), pass it in, then free it afterwards.
> >
> > So perhaps qemu_xen_xs_read() should be a printf-style function too,
> > with its last arg(s) being the node name.
>
> I just went with Anthony suggestion and introduced xs_node_read(), as
> I didn't want to play with qemu_xen_xs_read(). Not that I think the
> suggestion is not valid, just seemed more work than what I wanted to
> do right now.
Makes sense. Something like this¹?
char *xs_node_read(struct qemu_xs_handle *h, xs_transaction_t tid,
Error **errp, unsigned int *len,
const char *node_fmt, ...)
G_GNUC_PRINTF(5, 6);
There's a %ms in hw/xen/xen-block.c too, btw. Did you catch that one?
¹
https://git.infradead.org/?p=users/dwmw2/qemu.git;a=commitdiff;h=percentms;hp=bc6afa1c711da5b4f37c9685a812c77b114d84cb
smime.p7s
Description: S/MIME cryptographic signature