[PATCH 03/15] util: cgroup: Use GHashTable instead of virHashTable

[Pavel Hrdina]

On Thu, Oct 22, 2020 at 02:17:01PM +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 22, 2020 at 03:11:59PM +0200, Pavel Hrdina wrote:
> > On Thu, Oct 22, 2020 at 11:34:54AM +0200, Peter Krempa wrote:
> > > Rewrite using GHashTable which already has interfaces for using a number
> > > as hash key. Glib's implementation doesn't copy the key by default, so
> > > we need to allocate it, but overal the interface is more suited for this
> > > case.
> > > 
> > > Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> > > ---
> > >  src/util/vircgroup.c        | 61 ++++++++-----------------------------
> > >  src/util/vircgroupbackend.h |  3 +-
> > >  src/util/vircgrouppriv.h    |  2 +-
> > >  src/util/vircgroupv1.c      |  2 +-
> > >  src/util/vircgroupv2.c      |  2 +-
> > >  5 files changed, 17 insertions(+), 53 deletions(-)
> > > 
> > > diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
> > > index 5f4cb01bc0..b74ec1a8fa 100644
> > > --- a/src/util/vircgroup.c
> > > +++ b/src/util/vircgroup.c
> > > @@ -42,7 +42,6 @@
> > >  #include "virlog.h"
> > >  #include "virfile.h"
> > >  #include "virhash.h"
> > > -#include "virhashcode.h"
> > >  #include "virstring.h"
> > >  #include "virsystemd.h"
> > >  #include "virtypedparam.h"
> > > @@ -2382,7 +2381,7 @@ virCgroupRemove(virCgroupPtr group)
> > >  static int
> > >  virCgroupKillInternal(virCgroupPtr group,
> > >                        int signum,
> > > -                      virHashTablePtr pids,
> > > +                      GHashTable *pids,
> > >                        int controller,
> > >                        const char *taskFile)
> > >  {
> > > @@ -2415,8 +2414,9 @@ virCgroupKillInternal(virCgroupPtr group,
> > >              goto cleanup;
> > >          } else {
> > >              while (!feof(fp)) {
> > > -                long pid_value;
> > > -                if (fscanf(fp, "%ld", &pid_value) != 1) {
> > > +                g_autofree long long *pid_value = g_new0(long long, 1);
> > 
> > I would rather use gint64 here as the exact type of gint64 changes with
> > the hardware. For example on my AMD x86_84 it is 'signed long'.
> 
> If you use  gint64, then you need to start using PRIu64 macro
> to deal with the fact that the data type changes for printf
> formatting.
> 
> Using long long is simpler as you can unconditionally use  %ll
> which is a good thing IMHO.
> 
> 
> > We should do this every time we pass pointers to GLib APIs because for
> > example bool and gboolean are different and when I used bool type in
> > GLib dbus APIs it randomly crashed.
> 
> bool vs gboolean is a special case because of the different types.
> 
> For all the other g* basic types, we should not use them. GLib has
> a ticket open considering deprecating them, because they re-invent
> stdint.h

Good to know. I personally don't like these types as it complicates
thinks especially with the gboolean which is not that obvious.

I checked the GLib code and it handles it reasonably so using long long
should not be an issue.

Reviewed-by: Pavel Hrdina <phrdina at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201022/0fa34447/attachment-0001.sig>