Re: [PATCH 03/15] util: cgroup: Use GHashTable instead of virHashTable
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(a)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
>
> - VIR_DEBUG("pid=%ld", pid_value);
> + VIR_DEBUG("pid=%lld", *pid_value);
Using gint64 would require to use G_GINT64_FORMAT here.
That is exactly why we should not use guint64 - it makes the
debug code more verbose for no benefit.
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 :|