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'.
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.
> + if (fscanf(fp, "%lld", pid_value) != 1) {
> if (feof(fp))
> break;
> virReportSystemError(errno,
> @@ -2424,16 +2424,17 @@ virCgroupKillInternal(virCgroupPtr group,
> keypath);
> goto cleanup;
> }
> - if (virHashLookup(pids, (void*)pid_value))
> +
> + if (g_hash_table_lookup(pids, pid_value))
> continue;
>
> - VIR_DEBUG("pid=%ld", pid_value);
> + VIR_DEBUG("pid=%lld", *pid_value);
Using gint64 would require to use G_GINT64_FORMAT here.
Otherwise looks good.
Pavel
Attachment:
signature.asc
Description: PGP signature
