On Fri, Sep 27, 2019 at 06:17:29PM +0100, Daniel P. Berrangé wrote:
> Converting from virObject to GObject is reasonably straightforward,
> as illustrated by this patch for virIdentity
>
> In the header file
>
> - Remove
>
> typedef struct _virIdentity virIdentity
>
> - Add
>
> #define VIR_TYPE_IDENTITY virIdentity_get_type ()
> G_DECLARE_FINAL_TYPE (virIdentity, virIdentity, VIR, IDENTITY, GObject);
>
> Which provides the typedef we just removed, and class
> declaration boilerplate and various other constants/macros.
>
> In the source file
>
> - Change 'virObject parent' to 'GObject parent' in the struct
> - Remove the virClass variable and its initializing call
> - Add
>
> G_DEFINE_TYPE(virIdentity, virIdentity, G_TYPE_OBJECT)
>
> which declares the instance & class constructor functions
>
> - Add an impl of the instance & class constructors
> wiring up the finalize method to point to our dispose impl
>
> In all files
>
> - Replace VIR_AUTOUNREF(virIdentityPtr) with g_autoptr(virIdentity)
>
> - Replace virObjectRef/Unref with g_object_ref/unref. Note
> the latter functions do *NOT* accept a NULL object where as
> libvirt's do. If you replace g_object_unref with g_clear_object
> it is NULL safe, but also clears the pointer.
>
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
> src/access/viraccessdriverpolkit.c | 21 +++----
> src/admin/admin_server.c | 3 +-
> src/qemu/qemu_process.c | 4 +-
> src/remote/remote_daemon.c | 3 +-
> src/remote/remote_daemon_dispatch.c | 35 ++++--------
> src/rpc/virnetserverclient.c | 57 ++++++++-----------
> src/rpc/virnetserverprogram.c | 13 +----
> src/util/viridentity.c | 87 ++++++++++++++++-------------
> src/util/viridentity.h | 7 ++-
> tests/viridentitytest.c | 45 ++++++---------
> tests/virnetserverclienttest.c | 3 +-
> 11 files changed, 122 insertions(+), 156 deletions(-)
As Jan pointed out this patch should do the minimal conversion to
GObject to demonstrate the transition. Let's move the cleanup into
separate patch.
I'm OK with using g_autoptr for the new GObject as we don't have to
define anything else for it to work and that's what we want to
eventually use in our code base.
> diff --git a/src/util/viridentity.c b/src/util/viridentity.c
> index 22e2644c19..467d953e17 100644
> --- a/src/util/viridentity.c
> +++ b/src/util/viridentity.c
> @@ -43,25 +43,29 @@
> VIR_LOG_INIT("util.identity");
>
> struct _virIdentity {
> - virObject parent;
> + GObject parent;
>
> int nparams;
> int maxparams;
> virTypedParameterPtr params;
> };
>
> -static virClassPtr virIdentityClass;
> +G_DEFINE_TYPE(virIdentity, virIdentity, G_TYPE_OBJECT)
> +
> static virThreadLocal virIdentityCurrent;
>
> -static void virIdentityDispose(void *obj);
> +static void virIdentityDispose(GObject *obj);
We should rename it to virIdentityFinalize as there is a difference
between finalize and dispose in glib.
Dispose may be called multiple times and should only remove and clear
references to other objects for cyclic dependencies, finalize is called
only once and should actually free any resources.
> -static int virIdentityOnceInit(void)
> +static void virIdentityCurrentCleanup(void *ident)
> {
> - if (!VIR_CLASS_NEW(virIdentity, virClassForObject()))
> - return -1;
> + if (ident)
> + g_object_unref(ident);
> +}
>
> +static int virIdentityOnceInit(void)
> +{
> if (virThreadLocalInit(&virIdentityCurrent,
> - (virThreadLocalCleanup)virObjectUnref) < 0) {
> + virIdentityCurrentCleanup) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Cannot initialize thread local for current identity"));
> return -1;
> @@ -72,13 +76,24 @@ static int virIdentityOnceInit(void)
>
> VIR_ONCE_GLOBAL_INIT(virIdentity);
>
> +static void virIdentity_init(virIdentity *ident G_GNUC_UNUSED)
> +{
> +}
> +
> +static void virIdentity_class_init(virIdentityClass *klass)
> +{
> + GObjectClass *obj = G_OBJECT_CLASS(klass);
> +
> + obj->finalize = virIdentityDispose;
Here we correctly use finalize so lets match the function name to it as
well.
Otherwise looks good.
Pavel
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
