Re: [PATCH 1/1] all: add C99's "bool" define and encourage its use

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.


Since glib uses gboolean in quite a few places, would we have
with casting it to gboolean if needed?  Casting would be pretty ugly
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

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



 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
+ *       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 /* __NM_DEFAULT_H__ */

Attachment: signature.asc
Description: This is a digitally signed message part

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