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@xxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
