Build ell with clang

Build ell with clang

[ell]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

Hello,

the following patchseries allows ell to be build with clang.
For clang -Wcast-align has to be removed. I asked about this on
#llvm(a)irc.oftc.net and was told, this is the best option.
They also told me that gcc only actually produces a warning, if the
compilation target does not support unaligned access at all, so for x86
the option does nothing.
It may well be, that gcc will error out as well, if such an architecture
were to be targeted.
It may be better to remove -Wcast-align completely instead of making it
gcc specific, as this patch does.

Regards,
Markus

[PATCH 1/4] autotools: Only used -Wcast-align with gcc

[ell]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

From: Markus Ongyerth <ongy@ongy.net>

clang can currently not compile NLMSG_NEXT with -Wcast-align.
I asked about this in #llvm on irc.oftc.net, where I was told the best
solution is to disable -Wcast-align for now.
---
 acinclude.m4 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 8aab4ee..b1dbabe 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -58,6 +58,8 @@ AC_DEFUN([COMPILER_FLAGS], [
 		CFLAGS+=" -Wdeclaration-after-statement"
 		CFLAGS+=" -Wmissing-declarations"
 		CFLAGS+=" -Wredundant-decls"
-		CFLAGS+=" -Wcast-align"
+		if ( $CC -v 2>/dev/null | grep "gcc version" ); then
+			CFLAGS+=" -Wcast-align"
+		fi
 	fi
 ])
-- 
2.10.2

Re: [PATCH 1/4] autotools: Only used -Wcast-align with gcc

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 425 bytes --]

Hi Markus,

On 12/14/2016 04:51 PM, ell(a)ongy.net wrote:
> From: Markus Ongyerth <ongy@ongy.net>
>
> clang can currently not compile NLMSG_NEXT with -Wcast-align.
> I asked about this in #llvm on irc.oftc.net, where I was told the best
> solution is to disable -Wcast-align for now.
> ---
>  acinclude.m4 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

All four applied. Thanks!

Regards,
-Denis

Re: [PATCH 1/4] autotools: Only used -Wcast-align with gcc

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

Hi Markus,

> clang can currently not compile NLMSG_NEXT with -Wcast-align.
> I asked about this in #llvm on irc.oftc.net, where I was told the best
> solution is to disable -Wcast-align for now.

can we get a little bit more background on this. I had also some issues with native gcc usage on Raspberry PI. And yes, the netlink macros are some offenders.

> ---
> acinclude.m4 | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 8aab4ee..b1dbabe 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -58,6 +58,8 @@ AC_DEFUN([COMPILER_FLAGS], [
> 		CFLAGS+=" -Wdeclaration-after-statement"
> 		CFLAGS+=" -Wmissing-declarations"
> 		CFLAGS+=" -Wredundant-decls"
> -		CFLAGS+=" -Wcast-align"
> +		if ( $CC -v 2>/dev/null | grep "gcc version" ); then
> +			CFLAGS+=" -Wcast-align"
> +		fi

Are you sure there is not a better autoconf macro for checking if we use gcc vs clang.

Regards

Marcel

Re: [PATCH 1/4] autotools: Only used -Wcast-align with gcc

[Markus Ongyerth]

[-- Attachment #1: Type: text/plain, Size: 2758 bytes --]

Hi Marcel,

On 2016/12月/15 07:57, Marcel Holtmann wrote:
> Hi Markus,
> 
> > clang can currently not compile NLMSG_NEXT with -Wcast-align.
> > I asked about this in #llvm on irc.oftc.net, where I was told the best
> > solution is to disable -Wcast-align for now.
> 
> can we get a little bit more background on this. I had also some issues with native gcc usage on Raspberry PI. And yes, the netlink macros are some offenders.
> 
I was mostly going of an answer I got in irc, (I was told it's ok to quote):

/***************************************************************************/
2016-12-14 17:47:14	dblaikie	ongy: Judging by the docs - and this is a bit 
of a guess, GCC is more conservative with this warning, restricting it to only 
platforms where it would be an illegal instruction (I haven't actually tested 
this, because I don't have a GCC cross compiler and don't remember which 
targets have this behavior - x86 just has reduced performance)
/***************************************************************************/

The docs they reference here are probably [1] . Here the relevant lines are:

/***************************************************************************/
-Wcast-align
Warn whenever a pointer is cast such that the required alignment of the target
is increased. For example, warn if a char * is cast to an int * on machines 
where integers can only be accessed at two- or four-byte boundaries. 
/***************************************************************************/

Judging from [2] this is the case on older ARM processors.
If I read this correctly, The Raspberry PI should support unaligned access (at 
least version 2 and 3). It may be worth to see what architecture gcc targets 
and make it more specific.


[1] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.faqs/ka15414.html
> > ---
> > acinclude.m4 | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index 8aab4ee..b1dbabe 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -58,6 +58,8 @@ AC_DEFUN([COMPILER_FLAGS], [
> > 		CFLAGS+=" -Wdeclaration-after-statement"
> > 		CFLAGS+=" -Wmissing-declarations"
> > 		CFLAGS+=" -Wredundant-decls"
> > -		CFLAGS+=" -Wcast-align"
> > +		if ( $CC -v 2>/dev/null | grep "gcc version" ); then
> > +			CFLAGS+=" -Wcast-align"
> > +		fi
> 
> Are you sure there is not a better autoconf macro for checking if we use gcc vs clang.

No I'm not. I'm not really familiar with autotools. This is what came up when 
I searched for compiler specific option in autotools, there may be a better 
way.

Regards,
  Markus

[PATCH 2/4] dbus: Make adjustments for compilation with clang

[ell]

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]

From: Markus Ongyerth <ongy@ongy.net>

clang errors if KDBUS_ITEM_SIZE is a function, because it is used in an
array size inside a struct, where clang does not accept dynamic values.

clang warns about va_lists with elements of type char, since they are
subject to auto-expansion and will be converted to ints either way.
---
 ell/dbus-kernel.c  | 6 ++----
 ell/dbus-message.c | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/ell/dbus-kernel.c b/ell/dbus-kernel.c
index 6544a87..4d7cae3 100644
--- a/ell/dbus-kernel.c
+++ b/ell/dbus-kernel.c
@@ -58,10 +58,8 @@
         { 0x##v0, 0x##v1, 0x##v2, 0x##v3, 0x##v4, 0x##v5, 0x##v6, 0x##v7, \
 	0x##v8, 0x##v9, 0x##v10, 0x##v11, 0x##v12, 0x##v13, 0x##v14, 0x##v15 }
 
-static inline size_t KDBUS_ITEM_SIZE(size_t actual)
-{
-	return align_len(actual + offsetof(struct kdbus_item, data), 8);
-}
+#define KDBUS_ITEM_SIZE(actual) \
+	align_len(actual + offsetof(struct kdbus_item, data), 8)
 
 static inline struct kdbus_item *KDBUS_ITEM_NEXT(struct kdbus_item *item)
 {
diff --git a/ell/dbus-message.c b/ell/dbus-message.c
index 3b44fb8..ef5dd22 100644
--- a/ell/dbus-message.c
+++ b/ell/dbus-message.c
@@ -660,7 +660,7 @@ static bool get_header_field_from_iter_valist(struct l_dbus_message *message,
 }
 
 static inline bool get_header_field(struct l_dbus_message *message,
-					uint8_t type, char data_type, ...)
+					uint8_t type, int data_type, ...)
 {
 	va_list args;
 	bool result;
-- 
2.10.2

[PATCH 3/4] dbus: Add missing dereference on assign

[ell]

[-- Attachment #1: Type: text/plain, Size: 493 bytes --]

From: Markus Ongyerth <ongy@ongy.net>

---
 ell/dbus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ell/dbus.c b/ell/dbus.c
index 2c04ec9..5318794 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -480,7 +480,7 @@ static bool auth_read_handler(struct l_io *io, void *user_data)
 
 	l_util_hexdump(true, ptr, len, dbus->debug_handler, dbus->debug_data);
 
-	end = '\0';
+	*end = '\0';
 
 	switch (classic->auth_state) {
 	case WAITING_FOR_OK:
-- 
2.10.2

[PATCH 4/4] tls-record: remove unnecessary null-pointer check

[ell]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

From: Markus Ongyerth <ongy@ongy.net>

---
 ell/tls-record.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ell/tls-record.c b/ell/tls-record.c
index 3033372..c0e5834 100644
--- a/ell/tls-record.c
+++ b/ell/tls-record.c
@@ -371,8 +371,8 @@ static bool tls_handle_ciphertext(struct l_tls *tls)
 		tls_write_mac(tls, compressed + 8, 5 + compressed_len,
 				mac_buf, false);
 
-		if (tls->mac_length && memcmp(mac_buf, compressed + 13 +
-					compressed_len, tls->mac_length[0])) {
+		if (memcmp(mac_buf, compressed + 13 + compressed_len,
+							tls->mac_length[0])) {
 			tls_disconnect(tls, TLS_ALERT_BAD_RECORD_MAC, 0);
 			return false;
 		}
-- 
2.10.2