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__ */
Attachment:
signature.asc
Description: This is a digitally signed message part