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

2019-10-09 Thread 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é 
> > > > ---
> > > > 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

2019-10-09 Thread Ján Tomko 

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

2019-10-09 Thread Pavel Hrdina 
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

2019-10-08 Thread Ján Tomko 

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

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

2019-10-07 Thread Daniel P . Berrangé 
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/m4/virt-glib.m4 b/m4/virt-glib.m4
index 5a5bc19660..eb2c77b25b 100644
--- a/m4/virt-glib.m4
+++ b/m4/virt-glib.m4
@@ -24,10 +24,10 @@ AC_DEFUN([LIBVIRT_ARG_GLIB], [
 AC_DEFUN([LIBVIRT_CHECK_GLIB],[
   GLIB_REQUIRED=2.48.0
 
-  LIBVIRT_CHECK_PKG([GLIB], [glib-2.0], [$GLIB_REQUIRED])
+  LIBVIRT_CHECK_PKG([GLIB], [glib-2.0 gobject-2.0], [$GLIB_REQUIRED])
 
   if test "$with_glib" = "no" ; then
-AC_MSG_ERROR([glib-2.0 >= $GLIB_REQUIRED is required for libvirt])
+AC_MSG_ERROR([glib-2.0, gobject-2.0 >= $GLIB_REQUIRED are required for 
libvirt])
   fi
 ])
 
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a50cd54393..872750ff4d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -8051,7 +8051,7 @@ qemuProcessReconnect(void *opaque)
 bool tryMonReconn = false;
 
 virIdentitySetCurrent(data->identity);
-virObjectUnref(data->identity);
+g_clear_object(>identity);
 VIR_FREE(data);
 
 qemuDomainObjRestoreJob(obj, );
@@ -8364,7 +8364,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj,
 
 virDomainObjEndAPI();
 virNWFilterUnlockFilterUpdates();
-virObjectUnref(data->identity);
+g_clear_object(>identity);
 VIR_FREE(data);
 return -1;
 }
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 79287572b6..8482c5c29c 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -829,7 +829,7 @@ virIdentityPtr 
virNetServerClientGetIdentity(virNetServerClientPtr client)
 if (!client->identity)
 client->identity = virNetServerClientCreateIdentity(client);
 if (client->identity)
-ret = virObjectRef(client->identity);
+ret = g_object_ref(client->identity);
 virObjectUnlock(client);
 return ret;
 }
@@ -839,10 +839,10 @@ void virNetServerClientSetIdentity(virNetServerClientPtr 
client,
virIdentityPtr identity)
 {
 virObjectLock(client);
-virObjectUnref(client->identity);
+g_clear_object(>identity);
 client->identity = identity;
 if (client->identity)
-virObjectRef(client->identity);
+g_object_ref(client->identity);
 virObjectUnlock(client);
 }
 
@@ -979,7 +979,7 @@ void virNetServerClientDispose(void *obj)
 if (client->privateData)
 client->privateDataFreeFunc(client->privateData);
 
-virObjectUnref(client->identity);
+g_clear_object(>identity);
 
 #if WITH_SASL
 virObjectUnref(client->sasl);
@@ -1674,7 +1674,7 @@ virNetServerClientGetInfo(virNetServerClientPtr client,
 goto cleanup;
 }
 
-*identity = virObjectRef(client->identity);
+*identity = g_object_ref(client->identity);
 
 ret = 0;
  cleanup:
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 @@
 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 virIdentityFinalize(GObject *obj);
 
-static int virIdentityOnceInit(void)
+static void virIdentityCurrentCleanup(void *ident)
 {
-if