On Wed, 2015-11-11 at 11:00 -0600, Dan Williams wrote: > On Wed, 2015-11-11 at 13:33 +0100, Thomas Haller wrote: > > --- > > Let's use bool. > > > > Comments? > > Since glib uses gboolean in quite a few places, would we have > problems > with casting it to gboolean if needed? Casting would be pretty ugly > if > we had to do it a bunch.
No problem, no cast needed. Assigning gboolean to _Bool and vice versa
would always yield either 0 or 1 in the result. Contrary to assigning a
gboolean to a gboolean, which possibly yields a result in range
[G_MININT, G_MAXINT].
Also, setting a GObject property of type boolean will just work:
_Bool x = TRUE;
g_object_set (obj, NM_PROP_BOOL, x, NULL);
because the variadic argument type will be promoted to int.
Ok, the getter does require extra work:
gboolean tmp;
_Bool x;
g_object_get (obj, NM_PROP_BOOL, &tmp, NULL);
x = tmp;
> Also, I don't think we'd save any space because the compiler is going
> to
> pad any field 1-byte field (like bool) out to 4 or 8 bytes anyway,
> unless the structure is packed.
Obviously, you have to consider struct alignment for having any saving.
In the worst cast, it uses as much memory as gboolean.
> Lastly, using bitfields in structs is not generally recommended since
> apparently (at least) gcc creates awful code for accesses. At least
> that's the argument in kernel-land for not doing bitfields, and you
> never see them there.
I don't want to use now bitfields everywhere. But with gboolean you
*cannot*. With _Bool, whether a field is a bitfield or not has almost
no difference in how you can use it.
I'd like to see _Bool as an accepted datatype in NetworkManager. I
think it's preferable to gboolean in most situations, except for the
fact that gboolean is wildly used and by introducing _Bool you must be
aware of yet another type. But then, _Bool is hardly a complicated type
and even C99 standard.
Thomas
> > Thomas
> >
> > include/nm-default.h | 46
> > ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 46 insertions(+)
> >
> > diff --git a/include/nm-default.h b/include/nm-default.h
> > index d5c9d10..dc02763 100644
> > --- a/include/nm-default.h
> > +++ b/include/nm-default.h
> > @@ -70,4 +70,50 @@
> >
> > /*****************************************************************
> > ************/
> >
> > +/**
> > + * The boolean type _Bool is C99 but we mostly stick to C89.
> > However, _Bool is too
> > + * convinient to miss and is effectively available in gcc and
> > clang. So, just use it.
> > + *
> > + * Usually, one would include "stdbool.h" to get the bool define
> > which aliases
> > + * _Bool. We provide this define here, because we want to make use
> > of it anywhere.
> > + * (also, stdbool.h is again C99).
> > + *
> > + * Using _Bool has advantages over gboolean:
> > + *
> > + * - commonly, _Bool is one byte large, instead of gboolean's 4
> > bytes (because gboolean
> > + * is a typedef for gint). Especially when having boolean fields
> > in a struct, we can
> > + * thereby easily save some space.
> > + *
> > + * - _Bool type is guaranteed that two true expressions compare
> > equal. E.g. the follwing
> > + * will not work:
> > + * gboolean v1 = 1;
> > + * gboolean v2 = 2;
> > + * g_assert_cmpint (v1, ==, v2); // will fail
> > + * For that, we often to use !! to coerce gboolean values to 0
> > or 1:
> > + * g_assert_cmpint (!!v2, ==, TRUE);
> > + * With _Bool this will be properly done by the compiler.
> > + *
> > + * - For structs, we might want to safe even more space and use
> > bitfields:
> > + * struct s1 {
> > + * gboolean v1:1;
> > + * };
> > + * But the problem here is that gboolean is signed, so that
> > + * v1 will be either 0 or -1 (not 1, TRUE). Thus, the following
> > + * doesn't work:
> > + * struct s1 s = { .v1 = TRUE, };
> > + * g_assert_cmpint (s1.v1, ==, TRUE);
> > + * It will however work just fine with bool/_Bool.
> > + *
> > + * Also, add the defines for true/false. Those are nicely
> > highlighted by the editor
> > + * as special types, contrary to glib's TRUE/FALSE.
> > + */
> > +
> > +#ifndef bool
> > +#define bool _Bool
> > +#define true 1
> > +#define false 0
> > +#endif
> > +
> > +/*****************************************************************
> > ************/
> > +
> > #endif /* __NM_DEFAULT_H__ */
>
>
signature.asc
Description: This is a digitally signed message part
_______________________________________________ networkmanager-list mailing list networkmanager-list@gnome.org https://mail.gnome.org/mailman/listinfo/networkmanager-list
