On 3/2/22 13:33, Kristina Hanicova wrote:
> We should allow resetting and freeing qemuDomainJobObj even if
> 'cb' attribute is not set. This is theoretical for now, but the
> attribute must not be always set in the future, so early return
> would create memory leaks. It is sufficient to check if 'cb'
> exists before dereferencing it in qemuDomainObjClearJob() and
> also qemuDomainObjResetAsyncJob() as the latter is called from
> the former.
>
> This commit partially reverts af16e754cd4efc3ca1.
>
> Signed-off-by: Kristina Hanicova <khanicov@xxxxxxxxxx>
> ---
> src/qemu/qemu_domainjob.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_domainjob.c b/src/qemu/qemu_domainjob.c
> index 3e73eba4ed..587c166d94 100644
> --- a/src/qemu/qemu_domainjob.c
> +++ b/src/qemu/qemu_domainjob.c
> @@ -239,8 +239,10 @@ qemuDomainObjResetAsyncJob(qemuDomainJobObj *job)
> job->abortJob = false;
> VIR_FREE(job->error);
> g_clear_pointer(&job->current, virDomainJobDataFree);
> - job->cb->resetJobPrivate(job->privateData);
> job->apiFlags = 0;
> +
> + if (job->cb)
> + job->cb->resetJobPrivate(job->privateData);
> }
>
> int
> @@ -270,16 +272,15 @@ qemuDomainObjRestoreJob(virDomainObj *obj,
> void
> qemuDomainObjClearJob(qemuDomainJobObj *job)
> {
> - if (!job->cb)
> - return;
> -
> qemuDomainObjResetJob(job);
> qemuDomainObjResetAsyncJob(job);
> - g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
> g_clear_pointer(&job->current, virDomainJobDataFree);
> g_clear_pointer(&job->completed, virDomainJobDataFree);
> virCondDestroy(&job->cond);
> virCondDestroy(&job->asyncCond);
> +
> + if (job->cb)
> + g_clear_pointer(&job->privateData, job->cb->freeJobPrivate);
> }
>
> bool
This makes sense, and I'd just merge it. But I've got a question. So
this alloews job->cb to be NULL. But there are other functions which
takze job->cb and call a callback directly: qemuDomainObjRestoreJob(),
qemuDomainObjInitJob(), qemuDomainObjPrivateXMLFormatJob() and possibly
others. Would it make sense to threat them the same as you're doing here?
Michal
