[libvirt] [PATCH v2 15/23] util: convert virIdentity class to use GObject

[Daniel P. Berrangé]

On Wed, Oct 09, 2019 at 03:16:38PM +0200, Ján Tomko wrote:
> On Wed, Oct 09, 2019 at 09:07:57AM +0200, Pavel Hrdina wrote:
> > On Tue, Oct 08, 2019 at 07:08:25PM +0200, Ján Tomko wrote:
> > > On Mon, Oct 07, 2019 at 06:14:17PM +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 at redhat.com>
> > > > ---
> > > > m4/virt-glib.m4              |  4 +--
> > > > src/qemu/qemu_process.c      |  4 +--
> > > > src/rpc/virnetserverclient.c | 10 +++----
> > > > src/util/viridentity.c       | 57 ++++++++++++++++++++++--------------
> > > > src/util/viridentity.h       |  9 +++---
> > > > tests/viridentitytest.c      |  5 +---
> > > > 6 files changed, 50 insertions(+), 39 deletions(-)
> > > >
> 
> > > > /*
> > > >  * Returns: 0 if not present, 1 if present, -1 on error
> > > >  */
> > > > diff --git a/src/util/viridentity.h b/src/util/viridentity.h
> > > > index 7513dd4e35..df658ea844 100644
> > > > --- a/src/util/viridentity.h
> > > > +++ b/src/util/viridentity.h
> > > > @@ -21,12 +21,13 @@
> > > >
> > > > #pragma once
> > > >
> > > > -#include "virobject.h"
> > > > +#include "internal.h"
> > > > +#include <glib-object.h>
> > > >
> > > > -typedef struct _virIdentity virIdentity;
> > > > -typedef virIdentity *virIdentityPtr;
> > > > +#define VIR_TYPE_IDENTITY virIdentity_get_type()
> > > 
> > > Consider not mixing camel case with snake case.
> > 
> > In this case we have no choice, macro G_DEFINE_TYPE is responsible for
> > defining that function with type_name##_get_type where our call of that
> > macro uses virIdentity as type_name.
> > 
> 
> Well we have a choice of not doing that :)
> 
> Actually, G_DEFINE_TYPE is documented as taking one camelCase argument
> and one snake_case argument for the type name:
> https://developer.gnome.org/gobject/stable/gobject-Type-Information.html#G-DEFINE-TYPE:CAPS

If w do

G_DEFINE_TYPE(virIdentity, vir_identity, G_TYPE_OBJECT)

then we get  vir_identity_get_type() which is more normal glib
method naming style. Libvirt's method naming style is camelcase
though. Neither is perfect, but I guess glib style is less insane.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|