Re: [libvirt] [PATCH v2 15/23] util: convert virIdentity class to use GObject
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é > > > > --- > > > > 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 > > > > > > > > -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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 15/23] util: convert virIdentity class to use GObject
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é > --- > 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 > > -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 Jano Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 15/23] util: convert virIdentity class to use GObject
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é > > --- > > 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(-) > > > > diff --git a/src/util/viridentity.c b/src/util/viridentity.c > > index a24704b690..c6299a6b09 100644 > > --- a/src/util/viridentity.c > > +++ b/src/util/viridentity.c > > @@ -43,25 +43,29 @@ > > -static void virIdentityDispose(void *object) > > +static void virIdentityFinalize(GObject *object) > > { > > -virIdentityPtr ident = object; > > +virIdentityPtr ident = VIR_IDENTITY(object); > > > > virTypedParamsFree(ident->params, ident->nparams); > > + > > +G_OBJECT_CLASS(virIdentity_parent_class)->finalize(object); > > } > > > > > > + > > Extra whitespace > > > /* > > * 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 > > > > -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. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 15/23] util: convert virIdentity class to use GObject
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é --- 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(-) diff --git a/src/util/viridentity.c b/src/util/viridentity.c index a24704b690..c6299a6b09 100644 --- a/src/util/viridentity.c +++ b/src/util/viridentity.c @@ -43,25 +43,29 @@ -static void virIdentityDispose(void *object) +static void virIdentityFinalize(GObject *object) { -virIdentityPtr ident = object; +virIdentityPtr ident = VIR_IDENTITY(object); virTypedParamsFree(ident->params, ident->nparams); + +G_OBJECT_CLASS(virIdentity_parent_class)->finalize(object); } + Extra whitespace /* * 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 -typedef struct _virIdentity virIdentity; -typedef virIdentity *virIdentityPtr; +#define VIR_TYPE_IDENTITY virIdentity_get_type() Consider not mixing camel case with snake case. +G_DECLARE_FINAL_TYPE(virIdentity, virIdentity, VIR, IDENTITY, GObject); -G_DEFINE_AUTOPTR_CLEANUP_FUNC(virIdentity, virObjectUnref); +typedef virIdentity *virIdentityPtr; virIdentityPtr virIdentityGetCurrent(void); int virIdentitySetCurrent(virIdentityPtr ident); Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list