Re: [libvirt PATCH 3/3] qemuProcessReconnect: clear 'oldjob'

[Ján Tomko <jtomko@xxxxxxxxxx>]

On a Monday in 2020, Ján Tomko wrote:
> After we started copying the privateData pointer in
> qemuDomainObjRestoreJob, we should also free them
> once we're done with them.
>
> Register the clear function and use g_auto.
> Also add a check for job->cb to qemuDomainObjClearJob,
> to prevent freeing an uninitialized job.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1878450
>
> Signed-off-by: Ján Tomko <jtomko@xxxxxxxxxx>
> Fixes: aca37c3fb2e8d733c2788ca4b796c153ea7ce391
> ---
> src/qemu/qemu_domainjob.c | 3 +++
> src/qemu/qemu_domainjob.h | 1 +
> src/qemu/qemu_process.c   | 2 +-
> 3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
> index e5910a11a1..3c2c6b9179 100644
> --- a/src/qemu/qemu_domainjob.c
> +++ b/src/qemu/qemu_domainjob.c
> @@ -248,6 +248,9 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj,
> void
> qemuDomainObjClearJob(qemuDomainJobObjPtr job)
> {
> +    if (!job->cb)
> +        return;
> +
>     qemuDomainObjResetJob(job);
>     qemuDomainObjResetAsyncJob(job);
>     g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
> diff --git a/src/qemu/qemu_domainjob.h b/src/qemu/qemu_domainjob.h
> index eedd84c503..79f0127252 100644
> --- a/src/qemu/qemu_domainjob.h
> +++ b/src/qemu/qemu_domainjob.h
> @@ -275,6 +275,7 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
> bool qemuDomainTrackJob(qemuDomainJob job);
>
> void qemuDomainObjClearJob(qemuDomainJobObjPtr job);
> +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(qemuDomainJobObj, qemuDomainObjClearJob);
>
> int
> qemuDomainObjInitJob(qemuDomainJobObjPtr job,
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index b1af35b933..a345edd7fb 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -8081,7 +8081,7 @@ qemuProcessReconnect(void *opaque)
>     virQEMUDriverPtr driver = data->driver;
>     virDomainObjPtr obj = data->obj;
>     qemuDomainObjPrivatePtr priv;
> -    qemuDomainJobObj oldjob;
> +    g_auto(qemuDomainJobObj) oldjob = { 0 };

This fails on older compilers:
../src/qemu/qemu_process.c: In function ‘qemuProcessReconnect’:
../src/qemu/qemu_process.c:8084:5: error: missing braces around initializer [-Werror=missing-braces]
     g_auto(qemuDomainJobObj) oldjob = { 0 };
     ^

../src/qemu/qemu_process.c:8084:5: error: (near initialization for ‘oldjob.cond’) [-Werror=missing-braces]
../src/qemu/qemu_process.c:8084:5: error: missing initializer for field ‘active’ of ‘qemuDomainJobObj’ [-Werror=missing-field-initializers]
In file included from ../src/qemu/qemu_domain.h:34:0,
                 from ../src/qemu/qemu_process.h:25,
                 from ../src/qemu/qemu_process.c:41:
../src/qemu/qemu_domainjob.h:184:19: note: ‘active’ declared here
     qemuDomainJob active;               /* Currently running job */
                   ^
cc1: all warnings being treated as errors

conisder this squashed in:

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a345edd7fb..073b2d96e0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8081,7 +8081,9 @@ qemuProcessReconnect(void *opaque)
     virQEMUDriverPtr driver = data->driver;
     virDomainObjPtr obj = data->obj;
     qemuDomainObjPrivatePtr priv;
-    g_auto(qemuDomainJobObj) oldjob = { 0 };
+    g_auto(qemuDomainJobObj) oldjob = {
+      .cb = NULL,
+    };
     int state;
     int reason;
     g_autoptr(virQEMUDriverConfig) cfg = NULL;

Attachment: signature.asc
Description: PGP signature