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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Oct 22, 2020 at 14:17:01 +0100, Daniel Berrange 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(-)

[...]

> > > @@ -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.

Yup, simplicity of using %ll along with the fact that 'long long' is 64
bit signed integer on all of our supported platform lead me to use it
without trying to get into the glib types.




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux