On Thu, Oct 17, 2019 at 09:04:05 -0400, Cole Robinson wrote:
> On 10/16/19 4:54 PM, Daniel Henrique Barboza wrote:
> > This patch changes all virAsprintf calls to use the GLib API
> > g_strdup_printf in qemu_driver.c
> >
> > Signed-off-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>
> > ---
> > src/qemu/qemu_driver.c | 38 +++++++++++++++++---------------------
> > 1 file changed, 17 insertions(+), 21 deletions(-)
> >
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index a263393626..c9b3ed877f 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -402,7 +402,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
> >
> > priv = vm->privateData;
> >
> > - if (virAsprintf(&snapDir, "%s/%s", baseDir, vm->def->name) < 0) {
> > + if (!(snapDir = g_strdup_printf("%s/%s", baseDir, vm->def->name))) {
> > virReportError(VIR_ERR_INTERNAL_ERROR,
> > _("Failed to allocate memory for "
> > "snapshot directory for domain %s"),
> > @@ -427,7 +427,7 @@ qemuDomainSnapshotLoad(virDomainObjPtr vm,
> > kill the whole process */
> > VIR_INFO("Loading snapshot file '%s'", entry->d_name);
> >
>
> No objection to this patch specifically, but if g_strdup_printf is a
> drop in replacement for virAsprintf, I think conversion should be done
> in a mass change-the-world series like Jano has done for other pieces.
It actually is not a drop in replacement. virAsprintf is a wrapper
around g_vasprintf and aborts on OOM. On the other hand g_strdup_printf
returns NULL and doesn't set any error. I think our documentation is
wrong in suggesting we should use g_strdup_printf directly and I believe
this patch should be reverted.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
