[systemd-devel] [PATCH] Big endian and the size of the gcc builtin type bool from stdbool.h
On s390 the big endianness and cast from pointers of integers to
the type of bool leads to the funny status messages that e.g.
all targets are set to AllowIsolate=no even for multi-user.target.
The gcc builtin type bool or _Bool has the size of one byte which
should be taken into account in sd_bus_message_read_basic() as well
as in bus_message_append_ap()
---
src/libsystemd/sd-bus/bus-message.c | 19 +++
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git src/libsystemd/sd-bus/bus-message.c
src/libsystemd/sd-bus/bus-message.c
index bfb14fc..b70d814 100644
--- src/libsystemd/sd-bus/bus-message.c
+++ src/libsystemd/sd-bus/bus-message.c
@@ -2263,14 +2263,25 @@ int bus_message_append_ap(
r = sd_bus_message_append_basic(m, *t, x);
break;
}
+case SD_BUS_TYPE_BOOLEAN: {
+if (sizeof(bool) == sizeof(uint32_t)) {
+uint32_t x;
-case SD_BUS_TYPE_BOOLEAN:
+x = va_arg(ap, uint32_t);
+r = sd_bus_message_append_basic(m, *t, x);
+} else {
+uint8_t x;
+
+x = (uint8_t) va_arg(ap, int);
+r = sd_bus_message_append_basic(m, *t, x);
+}
+break;
+}
case SD_BUS_TYPE_INT32:
case SD_BUS_TYPE_UINT32:
case SD_BUS_TYPE_UNIX_FD: {
uint32_t x;
-/* We assume a boolean is the same as int32_t */
assert_cc(sizeof(int32_t) == sizeof(int));
x = va_arg(ap, uint32_t);
@@ -3233,7 +3244,7 @@ _public_ int sd_bus_message_read_basic(sd_bus_message *m,
char type, void *p) {
case SD_BUS_TYPE_BOOLEAN:
if (p)
-*(int*) p = !!*(uint8_t*) q;
+*(bool*) p = !!*(uint8_t*) q;
break;
case SD_BUS_TYPE_INT16:
@@ -3343,7 +3354,7 @@ _public_ int sd_bus_message_read_basic(sd_bus_message *m,
char type, void *p) {
case SD_BUS_TYPE_BOOLEAN:
if (p)
-*(int*) p = !!*(uint32_t*) q;
+*(bool*) p = !!*(uint32_t*) q;
break;
case SD_BUS_TYPE_INT16:
--
1.7.7
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Big endian and the size of the gcc builtin type bool from stdbool.h
Hi
On Thu, Sep 18, 2014 at 11:16 AM, Werner Fink wer...@suse.de wrote:
On s390 the big endianness and cast from pointers of integers to
the type of bool leads to the funny status messages that e.g.
all targets are set to AllowIsolate=no even for multi-user.target.
The gcc builtin type bool or _Bool has the size of one byte which
should be taken into account in sd_bus_message_read_basic() as well
as in bus_message_append_ap()
We don't support bool in public APIs. sd-bus requires you to use
int for boolean types. If a caller uses bool, they must be fixed.
Thanks
David
---
src/libsystemd/sd-bus/bus-message.c | 19 +++
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git src/libsystemd/sd-bus/bus-message.c
src/libsystemd/sd-bus/bus-message.c
index bfb14fc..b70d814 100644
--- src/libsystemd/sd-bus/bus-message.c
+++ src/libsystemd/sd-bus/bus-message.c
@@ -2263,14 +2263,25 @@ int bus_message_append_ap(
r = sd_bus_message_append_basic(m, *t, x);
break;
}
+case SD_BUS_TYPE_BOOLEAN: {
+if (sizeof(bool) == sizeof(uint32_t)) {
+uint32_t x;
-case SD_BUS_TYPE_BOOLEAN:
+x = va_arg(ap, uint32_t);
+r = sd_bus_message_append_basic(m, *t, x);
+} else {
+uint8_t x;
+
+x = (uint8_t) va_arg(ap, int);
+r = sd_bus_message_append_basic(m, *t, x);
+}
+break;
+}
case SD_BUS_TYPE_INT32:
case SD_BUS_TYPE_UINT32:
case SD_BUS_TYPE_UNIX_FD: {
uint32_t x;
-/* We assume a boolean is the same as int32_t */
assert_cc(sizeof(int32_t) == sizeof(int));
x = va_arg(ap, uint32_t);
@@ -3233,7 +3244,7 @@ _public_ int sd_bus_message_read_basic(sd_bus_message
*m, char type, void *p) {
case SD_BUS_TYPE_BOOLEAN:
if (p)
-*(int*) p = !!*(uint8_t*) q;
+*(bool*) p = !!*(uint8_t*) q;
break;
case SD_BUS_TYPE_INT16:
@@ -3343,7 +3354,7 @@ _public_ int sd_bus_message_read_basic(sd_bus_message
*m, char type, void *p) {
case SD_BUS_TYPE_BOOLEAN:
if (p)
-*(int*) p = !!*(uint32_t*) q;
+*(bool*) p = !!*(uint32_t*) q;
break;
case SD_BUS_TYPE_INT16:
--
1.7.7
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Big endian and the size of the gcc builtin type bool from stdbool.h
On Thu, Sep 18, 2014 at 12:43:00PM +0200, David Herrmann wrote:
Hi
On Thu, Sep 18, 2014 at 11:16 AM, Werner Fink wer...@suse.de wrote:
On s390 the big endianness and cast from pointers of integers to
the type of bool leads to the funny status messages that e.g.
all targets are set to AllowIsolate=no even for multi-user.target.
The gcc builtin type bool or _Bool has the size of one byte which
should be taken into account in sd_bus_message_read_basic() as well
as in bus_message_append_ap()
We don't support bool in public APIs. sd-bus requires you to use
int for boolean types. If a caller uses bool, they must be fixed.
Then I'd like to know why the header stdbool.h together with the
therein provided type _Bool aka bool is used overall systemd. Also
it is systemctl which does use sd_bus_message_read_basic() to parse
the answer received over sd_bus of the status question send over
sd_bus?
linuxadmin:~ # uname -m
s390x
linuxadmin:~ # systemctl show multi-user.target | grep -E '=(yes|no)'
CanStart=no
CanStop=no
CanReload=no
CanIsolate=no
StopWhenUnneeded=no
RefuseManualStart=no
RefuseManualStop=no
AllowIsolate=no
DefaultDependencies=no
IgnoreOnIsolate=no
IgnoreOnSnapshot=no
NeedDaemonReload=no
ConditionResult=no
Transient=no
Thanks
David
Werner
---
src/libsystemd/sd-bus/bus-message.c | 19 +++
1 files changed, 15 insertions(+), 4 deletions(-)
diff --git src/libsystemd/sd-bus/bus-message.c
src/libsystemd/sd-bus/bus-message.c
index bfb14fc..b70d814 100644
--- src/libsystemd/sd-bus/bus-message.c
+++ src/libsystemd/sd-bus/bus-message.c
@@ -2263,14 +2263,25 @@ int bus_message_append_ap(
r = sd_bus_message_append_basic(m, *t, x);
break;
}
+case SD_BUS_TYPE_BOOLEAN: {
+if (sizeof(bool) == sizeof(uint32_t)) {
+uint32_t x;
-case SD_BUS_TYPE_BOOLEAN:
+x = va_arg(ap, uint32_t);
+r = sd_bus_message_append_basic(m, *t, x);
+} else {
+uint8_t x;
+
+x = (uint8_t) va_arg(ap, int);
+r = sd_bus_message_append_basic(m, *t, x);
+}
+break;
+}
case SD_BUS_TYPE_INT32:
case SD_BUS_TYPE_UINT32:
case SD_BUS_TYPE_UNIX_FD: {
uint32_t x;
-/* We assume a boolean is the same as int32_t */
assert_cc(sizeof(int32_t) == sizeof(int));
x = va_arg(ap, uint32_t);
@@ -3233,7 +3244,7 @@ _public_ int sd_bus_message_read_basic(sd_bus_message
*m, char type, void *p) {
case SD_BUS_TYPE_BOOLEAN:
if (p)
-*(int*) p = !!*(uint8_t*) q;
+*(bool*) p = !!*(uint8_t*) q;
break;
case SD_BUS_TYPE_INT16:
@@ -3343,7 +3354,7 @@ _public_ int sd_bus_message_read_basic(sd_bus_message
*m, char type, void *p) {
case SD_BUS_TYPE_BOOLEAN:
if (p)
-*(int*) p = !!*(uint32_t*) q;
+*(bool*) p = !!*(uint32_t*) q;
break;
case SD_BUS_TYPE_INT16:
--
1.7.7
--
Having a smoking section in a restaurant is like having
a peeing section in a swimming pool. -- Edward Burr
pgp2fuPRMAUTb.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Big endian and the size of the gcc builtin type bool from stdbool.h
Hi On Thu, Sep 18, 2014 at 12:57 PM, Dr. Werner Fink wer...@suse.de wrote: On Thu, Sep 18, 2014 at 12:43:00PM +0200, David Herrmann wrote: Hi On Thu, Sep 18, 2014 at 11:16 AM, Werner Fink wer...@suse.de wrote: On s390 the big endianness and cast from pointers of integers to the type of bool leads to the funny status messages that e.g. all targets are set to AllowIsolate=no even for multi-user.target. The gcc builtin type bool or _Bool has the size of one byte which should be taken into account in sd_bus_message_read_basic() as well as in bus_message_append_ap() We don't support bool in public APIs. sd-bus requires you to use int for boolean types. If a caller uses bool, they must be fixed. Then I'd like to know why the header stdbool.h together with the therein provided type _Bool aka bool is used overall systemd. Also We use stdbool internally quite a lot. Especially bit-fields are really nice if implemented as bool. We just don't use bool in public APIs. We also avoid it in combination with va_arg() and with pointers, as in both cases the behavior is non-obvious and often done wrong. it is systemctl which does use sd_bus_message_read_basic() to parse the answer received over sd_bus of the status question send over sd_bus? All uses of sd-bus with bool are bugs and need to be fixed. See the history of bus-message.c: commit 9bcbce4201afada1c0ad8ada0cbfbbf58a52a6a7 Author: Kay Sievers k...@vrfy.org Date: Tue Oct 22 03:27:38 2013 +0200 bus: avoid 'bool' storage when retrieving 'b' from the message commit 102d8f8169427cb68cdebf5ee0f0e07788e9c2b2 Author: Kay Sievers k...@vrfy.org Date: Thu Nov 7 02:03:10 2013 +0100 consistently use int when retrieving bool from bus messages Thanks David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Big endian and the size of the gcc builtin type bool from stdbool.h
Hi On Thu, Sep 18, 2014 at 12:57 PM, Dr. Werner Fink wer...@suse.de wrote: linuxadmin:~ # uname -m s390x linuxadmin:~ # systemctl show multi-user.target | grep -E '=(yes|no)' CanStart=no CanStop=no CanReload=no CanIsolate=no StopWhenUnneeded=no RefuseManualStart=no RefuseManualStop=no AllowIsolate=no DefaultDependencies=no IgnoreOnIsolate=no IgnoreOnSnapshot=no NeedDaemonReload=no ConditionResult=no Transient=no I have a fix for bus_print_property() which is used by systemctl show. It used bool instead of int. I will push it in a moment. Thanks a lot for the report! David ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Big endian and the size of the gcc builtin type bool from stdbool.h
On Thu, Sep 18, 2014 at 01:20:47PM +0200, David Herrmann wrote: Hi On Thu, Sep 18, 2014 at 12:57 PM, Dr. Werner Fink wer...@suse.de wrote: On Thu, Sep 18, 2014 at 12:43:00PM +0200, David Herrmann wrote: Hi On Thu, Sep 18, 2014 at 11:16 AM, Werner Fink wer...@suse.de wrote: On s390 the big endianness and cast from pointers of integers to the type of bool leads to the funny status messages that e.g. all targets are set to AllowIsolate=no even for multi-user.target. The gcc builtin type bool or _Bool has the size of one byte which should be taken into account in sd_bus_message_read_basic() as well as in bus_message_append_ap() We don't support bool in public APIs. sd-bus requires you to use int for boolean types. If a caller uses bool, they must be fixed. Then I'd like to know why the header stdbool.h together with the therein provided type _Bool aka bool is used overall systemd. Also We use stdbool internally quite a lot. Especially bit-fields are really nice if implemented as bool. We just don't use bool in public APIs. We also avoid it in combination with va_arg() and with pointers, as in both cases the behavior is non-obvious and often done wrong. Hmmm ... I'm familiar with big endian (IMHO the natural byte order:) it is systemctl which does use sd_bus_message_read_basic() to parse the answer received over sd_bus of the status question send over sd_bus? All uses of sd-bus with bool are bugs and need to be fixed. See the history of bus-message.c: commit 9bcbce4201afada1c0ad8ada0cbfbbf58a52a6a7 Author: Kay Sievers k...@vrfy.org Date: Tue Oct 22 03:27:38 2013 +0200 bus: avoid 'bool' storage when retrieving 'b' from the message commit 102d8f8169427cb68cdebf5ee0f0e07788e9c2b2 Author: Kay Sievers k...@vrfy.org Date: Thu Nov 7 02:03:10 2013 +0100 consistently use int when retrieving bool from bus messages The only problem which remains: on s390 the sd_bus_message_read_basic() does it internal wrong (IMHO). If a bool is internal used and send over the dbus then the internal of sd_bus_message_read_basic() or at least bus_print_property() should be able to handle this. Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr pgp8RKPL_DNxY.pgp Description: PGP signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH] Big endian and the size of the gcc builtin type bool from stdbool.h
On Thu, Sep 18, 2014 at 01:29:26PM +0200, David Herrmann wrote: Hi On Thu, Sep 18, 2014 at 12:57 PM, Dr. Werner Fink wer...@suse.de wrote: linuxadmin:~ # uname -m s390x linuxadmin:~ # systemctl show multi-user.target | grep -E '=(yes|no)' CanStart=no CanStop=no CanReload=no CanIsolate=no StopWhenUnneeded=no RefuseManualStart=no RefuseManualStop=no AllowIsolate=no DefaultDependencies=no IgnoreOnIsolate=no IgnoreOnSnapshot=no NeedDaemonReload=no ConditionResult=no Transient=no I have a fix for bus_print_property() which is used by systemctl show. It used bool instead of int. I will push it in a moment. OK you're faster, thanks a lot :) Werner -- Having a smoking section in a restaurant is like having a peeing section in a swimming pool. -- Edward Burr pgpKZKqRGKc68.pgp Description: PGP signature ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
