C99 related fixes

This set of patches addresses some issues related to C library features that were disabled, or were at least affected, by explicitly enabling C99 in gcc via the -std=c99 command line option.

ELL itself won't build cleanly when constrained solely to C99. A successful build in that case requires defining the appropriate feature test macro(s) (e.g. _POSIX_SOURCE, _POSIX_C_SOURCE, etc) either in ELL's config.h header, the build command line, or individual source files that require the disabled C library feature.

Ossama Othman (5):
ell: Use __typeof__ instead of typeof in headers.
settings: Include strings.h for strcasecmp() proto
dbus-client: Only include required ELL headers.
ell: Make tls-private.h header self-contained.
tools: Only include required ELL headers.

 ell/cipher.h             | 1 +
 ell/dbus-client.c        | 4 +++-
 ell/private.h            | 2 +-
 ell/settings.c           | 1 +
 ell/tls-private.h        | 4 ++++
 ell/util.h               | 8 ++++----
 tools/certchain-verify.c | 5 ++++-
 7 files changed, 18 insertions(+), 7 deletions(-)

2.17.1

The typeof keyword is disabled by gcc when compiling with the -ansi or -std command line options. Use __typeof__ instead. This is particularly important for headers exposed to users that contain uses of typeof.

---
 ell/private.h | 2 +-
 ell/util.h    | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

index a77f677..f132577 100644
@@ -28,7 +28,7 @@
 #define uninitialized_var(x) x = x
 
 #define container_of(ptr, type, member) ({			\
-	const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
+	const __typeof__( ((type *)0)->member ) *__mptr = (ptr);	\
 	(type *)( (char *)__mptr - offsetof(type,member) );})
 
 #define align_len(len, boundary) (((len)+(boundary)-1) & ~((boundary)-1))

index 5021300..381721f 100644
@@ -59,16 +59,16 @@ extern "C" {
 #define L_GET_UNALIGNED(ptr) __extension__	\
 ({						\
 	struct __attribute__((packed)) {	\
-		typeof(*(ptr)) __v;		\
-	} *__p = (typeof(__p)) (ptr);		\
+            __typeof__(*(ptr)) __v;		\
+	} *__p = (__typeof__(__p)) (ptr);	\
 	__p->__v;				\
 })
 
 #define L_PUT_UNALIGNED(val, ptr)		\
 do {						\
 	struct __attribute__((packed)) {	\
-		typeof(*(ptr)) __v;		\
-	} *__p = (typeof(__p)) (ptr);		\
+		__typeof__(*(ptr)) __v;		\
+	} *__p = (__typeof__(__p)) (ptr);	\
 	__p->__v = (val);			\
 } while(0)
 

2.17.1

Hi Ossama,

patch has been applied.

Regards

Marcel

Restricting the C standard supported by compiler, e.g. to C99, can prevent some function prototypes from being indirectly pulled in, as is the case for strcasecmp(). Include <strings.h> to explicitly pull in its prototype.

---
 ell/settings.c | 1 +
 1 file changed, 1 insertion(+)

index 539263b..08a7748 100644
@@ -26,6 +26,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <strings.h>  // strcasecmp
 #include <errno.h>
 #include <sys/types.h>
 #include <sys/stat.h>

2.17.1

Hi Ossama,

Reduce dependencies in ell/dbus-client.c by only including the ELL headers it requires rather than the <ell/ell.h> convenience header. This also limits build side effects caused by enabling or disabling C library features through C standard related compiler flags (e.g. -std=c99).

---
 ell/dbus-client.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index 1628b67..d2eb70f 100644
@@ -25,7 +25,9 @@
 #include <config.h>
 #endif
 
-#include "ell.h"
+#include "dbus.h"
+#include "dbus-client.h"
+#include "queue.h"
 #include "private.h"
 
 struct l_dbus_client {

2.17.1

Hi Ossama,

Applied, thanks.

Regards,
-Denis

Add a missing struct iovec forward declaration, and include the appropriate headers in tls-private.h to make it self-contained. This simplifies its use so that files that include it need not be forced to include other headers it requires.

---
 ell/cipher.h      | 1 +
 ell/tls-private.h | 4 ++++
 2 files changed, 5 insertions(+)

index d787f33..6b43458 100644
@@ -27,6 +27,7 @@
 extern "C" {
 #endif
 
+struct iovec;
 struct l_cipher;
 
 enum l_cipher_type {

index 8cf5be9..0da7d21 100644
@@ -20,6 +20,10 @@
  *
  */
 
+#include <ell/checksum.h>
+#include <ell/cipher.h>
+#include <ell/tls.h>
+
 /* Only TLS 1.2 supported */
 #define TLS_V12		((3 << 8) | 3)
 #define TLS_V11		((3 << 8) | 2)

2.17.1

Hi Ossama,

Hi Denis,

It's used in tools/certchain-verify.c, but I'm not sure why it's used there since I wasn't involved with its implementation. I ran across several warnings and errors when attempting to build ELL with -std=c99 (not the norm, I know), some of which could only be corrected by making sure the appropriate feature test macro was defined by before including some ELL and system headers, e.g.:

CC tools/certchain-verify.o
In file included from ./ell/ell.h:32:0,
from tools/certchain-verify.c:39:
./ell/signal.h:39:40: error: unknown type name ‘sigset_t’
struct l_signal *l_signal_create(const sigset_t *mask,

                                        ^~~~~~~~

Makefile:1621: recipe for target 'tools/certchain-verify.o' failed

I don't need to build ELL with C99, and only attempted to do so when attempting to track down C99 related build errors in my own
application. In any case, replacing the <ell/ell.h> include
directive with only the required ELL headers corrected the above error, but I then ended up with other warnings and errors:

CC tools/certchain-verify.o
In file included from tools/certchain-verify.c:39:0:
./ell/tls-private.h:41:21: error: field ‘l_id’ has incomplete type

  enum l_cipher_type l_id;
                     ^~~~
./ell/tls-private.h:49:23: error: field ‘l_id’ has incomplete type
  enum l_checksum_type l_id;
                       ^~~~

./ell/tls-private.h:53:39: warning: ‘struct l_tls’ declared inside parameter list will not be visible outside of this definition or declaration
typedef bool (*tls_get_hash_t)(struct l_tls *tls, uint8_t tls_id,

                                       ^~~~~
./ell/tls-private.h:64:42: warning: ‘struct l_tls’ declared inside
parameter list will not be visible outside of this definition or
declaration
  bool (*send_client_key_exchange)(struct l_tls *tls);
                                          ^~~~~

./ell/tls-private.h:65:44: warning: ‘struct l_tls’ declared inside parameter list will not be visible outside of this definition or declaration
void (*handle_client_key_exchange)(struct l_tls *tls,

                                            ^~~~~

./ell/tls-private.h:68:25: warning: ‘struct l_tls’ declared inside parameter list will not be visible outside of this definition or declaration
ssize_t (*sign)(struct l_tls *tls, uint8_t *out, size_t len,

                         ^~~~~

./ell/tls-private.h:70:24: warning: ‘struct l_tls’ declared inside parameter list will not be visible outside of this definition or declaration
bool (*verify)(struct l_tls *tls, const uint8_t *in, size_t len,

                        ^~~~~
./ell/tls-private.h:76:23: error: field ‘hmac_type’ has incomplete type
  enum l_checksum_type hmac_type;
                       ^~~~~~~~~

./ell/tls-private.h:135:2: error: unknown type name ‘l_tls_write_cb_t’
l_tls_write_cb_t tx, rx;
^~~~~~~~~~~~~~~~
./ell/tls-private.h:136:2: error: unknown type name ‘l_tls_ready_cb_t’
l_tls_ready_cb_t ready_handle;
^~~~~~~~~~~~~~~~
./ell/tls-private.h:137:2: error: unknown type name ‘l_tls_disconnect_cb_t’
l_tls_disconnect_cb_t disconnected;
^~~~~~~~~~~~~~~~~~~~~
./ell/tls-private.h:216:45: warning: ‘enum l_tls_alert_desc’ declared inside parameter list will not be visible outside of this definition or declaration
void tls_disconnect(struct l_tls *tls, enum l_tls_alert_desc desc,

                                             ^~~~~~~~~~~~~~~~

Makefile:1621: recipe for target 'tools/certchain-verify.o' failed

This patch corrects these build warnings and errors. However, I understand your point about the header being private, so I won't push for this patch any further.

Thanks,
-Ossama

Limit build side effects caused by enabling or disabling C library features through C standard related compiler flags (e.g. -std=c99) by only including the required ELL headers.

---
 tools/certchain-verify.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

index 1800f68..e8d8374 100644
@@ -26,13 +26,16 @@
 #include <errno.h>
 #include <stdint.h>
 #include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <sys/mman.h>
 
-#include <ell/ell.h>
+#include <ell/log.h>
+#include <ell/util.h>
 #include "ell/tls-private.h"
 
 static int load_cert_chain(const char *file, struct tls_cert **certchain)

2.17.1

Hi Ossama,

Hi Ossama,

so besides including two of your patches, I fixed the rest by adding the correct headers and using _GNU_SOURCE to active the features. So with -std99 we should be fine now. I have to look into -ansi and if that is worth while supporting. However I hope that I didn’t break things for the musl builds.

Regards

Marcel

Hi Marcel,

Thanks for making those changes. ELL itself now builds with -std=c99 for me. However, the __sigset_t type used in <ell/signal.h> is still flagged as an unknown type in my code when compiling with -std=c99, without explicitly defining any feature test macros. For example, a trivial code snippet that only includes <ell/ell.h> fails:

foo.c:
#include <ell/ell.h>

$ gcc -std=c99 -fsyntax-only foo.c
In file included from /usr/local/include/ell/ell.h:32:0,
                from foo.c:1:
/usr/local/include/ell/signal.h:39:40: error: unknown type name ‘__sigset_t’
struct l_signal *l_signal_create(const __sigset_t *mask,
                                       ^~~~~~~~~~

Should the appropriate feature test macro be added to ELL's Cflags variable in the ell.pc pkg-config file?

Thanks,
-Ossama

Hi Ossama,

not in the pkg-config since that should really just only list include paths and library dependencies. What test feature is this behind anyway?

Also I am debating to actually change the API anyway and use ELL specific values for the signals. I think that exposing sigset_t is not a good idea. In wired/dbus.c of iwd, I have a full wrapper around this anyway and with the addition of exposing SIGUSR1, SIGUSR2 and SIGHUP, I doubt any application should deal with any other signals. At least not from a daemon perspective. There might be SIGCHLD if it spawns (which we discourage since we want to stay single process), but that we handled internally with a proper API anyway. Since child reaping is something really hard to get right so that code better be only written once anyway.

Regards

Marcel

Hi Marcel,

In glibc, it looks like sigset_t requires __USE_POSIX, which is defined when _POSIX_SOURCE, _POSIX_C_SOURCE >=1, or _XOPEN_SOURCE is defined. From glibc's <features.h>:

#if (defined _POSIX_SOURCE \
     || (defined _POSIX_C_SOURCE && _POSIX_C_SOURCE >= 1) \
     || defined _XOPEN_SOURCE)
# define __USE_POSIX 1
#endif

That sounds like a good plan.

Thanks,
-Ossama