[PATCH 0/5] Support C89 ELL builds

[PATCH 0/5] Support C89 ELL builds

[Ossama Othman]

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

This patch set provides several simple changes to enable C89 support
in ELL.

Ossama Othman (5):
  ell: Use __inline__ instead of inline for C89.
  ell: Do not use C99 '//' single line comment.
  settings: Explicitly enable strto{u}ll() for C89.
  string: Use __va_copy() for pre-C99 builds.
  unit: Remove C99 loop variable declaration.

 ell/asn1-private.h       | 4 ++++
 ell/dbus.c               | 2 +-
 ell/netlink.c            | 2 +-
 ell/settings.c           | 4 ++++
 ell/string.c             | 4 ++++
 ell/utf8.h               | 4 ++++
 ell/util.h               | 4 ++++
 unit/test-dbus-message.c | 3 ++-
 8 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH 1/5] ell: Use __inline__ instead of inline for C89.

[Ossama Othman]

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

The inline keyword was introduced in C99.  Use the gcc __inline__
extension for older C standards.
---
 ell/asn1-private.h | 4 ++++
 ell/utf8.h         | 4 ++++
 ell/util.h         | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/ell/asn1-private.h b/ell/asn1-private.h
index bea9ea9..53bba4c 100644
--- a/ell/asn1-private.h
+++ b/ell/asn1-private.h
@@ -40,6 +40,10 @@ struct asn1_oid {
 	((oid1)->asn1_len == (oid2_len) && \
 	 !memcmp((oid1)->asn1, (oid2_string), (oid2_len)))
 
+#if __STDC_VERSION__ <= 199409L
+#define inline __inline__
+#endif
+
 static inline int asn1_parse_definite_length(const uint8_t **buf,
 						size_t *len)
 {
diff --git a/ell/utf8.h b/ell/utf8.h
index 92226d3..c92da60 100644
--- a/ell/utf8.h
+++ b/ell/utf8.h
@@ -80,6 +80,10 @@ enum l_ascii {
 #define l_ascii_isxdigit(c) \
 	((l_ascii_table[(unsigned char) (c)] & L_ASCII_XDIGIT) != 0)
 
+#if __STDC_VERSION__ <= 199409L
+#define inline __inline__
+#endif
+
 static inline __attribute__ ((always_inline))
 					bool l_ascii_isblank(unsigned char c)
 {
diff --git a/ell/util.h b/ell/util.h
index de4df74..c4ba7f8 100644
--- a/ell/util.h
+++ b/ell/util.h
@@ -102,6 +102,10 @@ do {						\
 #error "Unknown byte order"
 #endif
 
+#if __STDC_VERSION__ <= 199409L
+#define inline __inline__
+#endif
+
 static inline uint8_t l_get_u8(const void *ptr)
 {
 	return *((const uint8_t *) ptr);
-- 
2.17.1

[PATCH 2/5] ell: Do not use C99 '//' single line comment.

[Ossama Othman]

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

Support older C standards by avoiding use of C99/C++ style '//'
comments.
---
 ell/dbus.c    | 2 +-
 ell/netlink.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ell/dbus.c b/ell/dbus.c
index 5664f85..92be396 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1286,7 +1286,7 @@ LIB_EXPORT bool l_dbus_set_debug(struct l_dbus *dbus,
 	dbus->debug_destroy = destroy;
 	dbus->debug_data = user_data;
 
-	//l_io_set_debug(dbus->io, function, user_data, NULL);
+	/* l_io_set_debug(dbus->io, function, user_data, NULL); */
 
 	return true;
 }
diff --git a/ell/netlink.c b/ell/netlink.c
index 39e6b3a..3d291cf 100644
--- a/ell/netlink.c
+++ b/ell/netlink.c
@@ -607,7 +607,7 @@ LIB_EXPORT bool l_netlink_set_debug(struct l_netlink *netlink,
 	netlink->debug_destroy = destroy;
 	netlink->debug_data = user_data;
 
-	//l_io_set_debug(netlink->io, function, user_data, NULL);
+	/* l_io_set_debug(netlink->io, function, user_data, NULL); */
 
 	return true;
 }
-- 
2.17.1

[PATCH 3/5] settings: Explicitly enable strto{u}ll() for C89.

[Ossama Othman]

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

The strtoll() and strtoull() functions were introduced in C99.  Define
_DEFAULT_SOURCE to enable them when building ELL with older C
standards, such as C89.  This addresses a run-time failure in
unit/test-settings when setting and getting 64 bit integers.
---
 ell/settings.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ell/settings.c b/ell/settings.c
index 0c06fe7..98823a2 100644
--- a/ell/settings.c
+++ b/ell/settings.c
@@ -24,6 +24,10 @@
 #include <config.h>
 #endif
 
+#if __STDC_VERSION__ <= 199409L
+#define _DEFAULT_SOURCE  /* for strto{u}ll() */
+#endif
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <strings.h>
-- 
2.17.1

[PATCH 4/5] string: Use __va_copy() for pre-C99 builds.

[Ossama Othman]

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

The va_copy() function was introduced in C99.  Use __va_copy() when
building for older C standards to address a link-time error.
---
 ell/string.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/ell/string.c b/ell/string.c
index f959bb9..37806e3 100644
--- a/ell/string.c
+++ b/ell/string.c
@@ -242,7 +242,11 @@ LIB_EXPORT void l_string_append_vprintf(struct l_string *dest,
 	if (unlikely(!dest))
 		return;
 
+#if __STDC_VERSION__ > 199409L
 	va_copy(args_copy, args);
+#else
+	__va_copy(args_copy, args);
+#endif
 
 	have_space = dest->max - dest->len;
 	len = vsnprintf(dest->str + dest->len, have_space, format, args);
-- 
2.17.1

[PATCH 5/5] unit: Remove C99 loop variable declaration.

[Ossama Othman]

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

---
 unit/test-dbus-message.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/unit/test-dbus-message.c b/unit/test-dbus-message.c
index f392c0e..c2ca128 100644
--- a/unit/test-dbus-message.c
+++ b/unit/test-dbus-message.c
@@ -1822,6 +1822,7 @@ static void parse_struct_3(const void *data)
 	struct l_dbus_message *msg = check_message(data);
 	const int num[32] = { 0 };
 	bool result;
+	int i;
 
 	result = l_dbus_message_get_arguments(msg, MESSAGE_SIGNATURE_STRUCT_3,
 		      &num[0], &num[1], &num[2], &num[3],
@@ -1835,7 +1836,7 @@ static void parse_struct_3(const void *data)
 
 	assert(result);
 
-	for (int i = 0; i < 32; i++) {
+	for (i = 0; i < 32; i++) {
 		assert(i == num[i]);
 	}
 
-- 
2.17.1

Re: [PATCH 0/5] Support C89 ELL builds

[Denis Kenzior]

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

Hi Ossama,

On 03/04/2019 12:42 PM, Ossama Othman wrote:
> This patch set provides several simple changes to enable C89 support
> in ELL.
> 
> Ossama Othman (5):
>    ell: Use __inline__ instead of inline for C89.
>    ell: Do not use C99 '//' single line comment.
>    settings: Explicitly enable strto{u}ll() for C89.
>    string: Use __va_copy() for pre-C99 builds.
>    unit: Remove C99 loop variable declaration.
> 
>   ell/asn1-private.h       | 4 ++++
>   ell/dbus.c               | 2 +-
>   ell/netlink.c            | 2 +-
>   ell/settings.c           | 4 ++++
>   ell/string.c             | 4 ++++
>   ell/utf8.h               | 4 ++++
>   ell/util.h               | 4 ++++
>   unit/test-dbus-message.c | 3 ++-
>   8 files changed, 24 insertions(+), 3 deletions(-)
> 

All applied, thanks.

Regards,
-Denis

Re: [PATCH 0/5] Support C89 ELL builds

[=?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek]

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

Just a question: is C89 compatibility a goal? C99 is nearly 20 y.o. I can't
see a reason to keep this, while at the same time target Linux.

On Mon, 4 Mar 2019, 20:23 Denis Kenzior, <denkenz@gmail.com> wrote:

> Hi Ossama,
>
> On 03/04/2019 12:42 PM, Ossama Othman wrote:
> > This patch set provides several simple changes to enable C89 support
> > in ELL.
> >
> > Ossama Othman (5):
> >    ell: Use __inline__ instead of inline for C89.
> >    ell: Do not use C99 '//' single line comment.
> >    settings: Explicitly enable strto{u}ll() for C89.
> >    string: Use __va_copy() for pre-C99 builds.
> >    unit: Remove C99 loop variable declaration.
> >
> >   ell/asn1-private.h       | 4 ++++
> >   ell/dbus.c               | 2 +-
> >   ell/netlink.c            | 2 +-
> >   ell/settings.c           | 4 ++++
> >   ell/string.c             | 4 ++++
> >   ell/utf8.h               | 4 ++++
> >   ell/util.h               | 4 ++++
> >   unit/test-dbus-message.c | 3 ++-
> >   8 files changed, 24 insertions(+), 3 deletions(-)
> >
>
> All applied, thanks.
>
> Regards,
> -Denis
> _______________________________________________
> ell mailing list
> ell(a)lists.01.org
> https://lists.01.org/mailman/listinfo/ell
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1854 bytes --]

Re: [PATCH 0/5] Support C89 ELL builds

[Othman, Ossama]

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

Hi Michał,

Part of the motivation of this patch set was to make the code more closely
conform to ELL's coding style, which disallows some C99 features.
Explicitly building ELL with C89 (CFLAGS=-std=c89) helped expose some of
style violations.  Getting ELL to work with C89 was also trivial, so I
submitted patches accordingly.

-Ossama

On Mon, Mar 4, 2019 at 1:15 PM Michał Lowas-Rzechonek <
michal.lowas-rzechonek@silvair.com> wrote:

> Just a question: is C89 compatibility a goal? C99 is nearly 20 y.o. I
> can't see a reason to keep this, while at the same time target Linux.
>
> On Mon, 4 Mar 2019, 20:23 Denis Kenzior, <denkenz@gmail.com> wrote:
>
>> Hi Ossama,
>>
>> On 03/04/2019 12:42 PM, Ossama Othman wrote:
>> > This patch set provides several simple changes to enable C89 support
>> > in ELL.
>> >
>> > Ossama Othman (5):
>> >    ell: Use __inline__ instead of inline for C89.
>> >    ell: Do not use C99 '//' single line comment.
>> >    settings: Explicitly enable strto{u}ll() for C89.
>> >    string: Use __va_copy() for pre-C99 builds.
>> >    unit: Remove C99 loop variable declaration.
>> >
>> >   ell/asn1-private.h       | 4 ++++
>> >   ell/dbus.c               | 2 +-
>> >   ell/netlink.c            | 2 +-
>> >   ell/settings.c           | 4 ++++
>> >   ell/string.c             | 4 ++++
>> >   ell/utf8.h               | 4 ++++
>> >   ell/util.h               | 4 ++++
>> >   unit/test-dbus-message.c | 3 ++-
>> >   8 files changed, 24 insertions(+), 3 deletions(-)
>> >
>>
>> All applied, thanks.
>>
>> Regards,
>> -Denis
>> _______________________________________________
>> ell mailing list
>> ell(a)lists.01.org
>> https://lists.01.org/mailman/listinfo/ell
>>
>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 2692 bytes --]

Re: [PATCH 0/5] Support C89 ELL builds

[=?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek]

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

Yeah, I get that! My question was mostly to Denis, about the overall policy.

On Mon, 4 Mar 2019, 22:58 Othman, Ossama, <ossama.othman@intel.com> wrote:

> Hi Michał,
>
> Part of the motivation of this patch set was to make the code more closely
> conform to ELL's coding style, which disallows some C99 features.
> Explicitly building ELL with C89 (CFLAGS=-std=c89) helped expose some of
> style violations.  Getting ELL to work with C89 was also trivial, so I
> submitted patches accordingly.
>
> -Ossama
>
> On Mon, Mar 4, 2019 at 1:15 PM Michał Lowas-Rzechonek <
> michal.lowas-rzechonek(a)silvair.com> wrote:
>
>> Just a question: is C89 compatibility a goal? C99 is nearly 20 y.o. I
>> can't see a reason to keep this, while at the same time target Linux.
>>
>> On Mon, 4 Mar 2019, 20:23 Denis Kenzior, <denkenz@gmail.com> wrote:
>>
>>> Hi Ossama,
>>>
>>> On 03/04/2019 12:42 PM, Ossama Othman wrote:
>>> > This patch set provides several simple changes to enable C89 support
>>> > in ELL.
>>> >
>>> > Ossama Othman (5):
>>> >    ell: Use __inline__ instead of inline for C89.
>>> >    ell: Do not use C99 '//' single line comment.
>>> >    settings: Explicitly enable strto{u}ll() for C89.
>>> >    string: Use __va_copy() for pre-C99 builds.
>>> >    unit: Remove C99 loop variable declaration.
>>> >
>>> >   ell/asn1-private.h       | 4 ++++
>>> >   ell/dbus.c               | 2 +-
>>> >   ell/netlink.c            | 2 +-
>>> >   ell/settings.c           | 4 ++++
>>> >   ell/string.c             | 4 ++++
>>> >   ell/utf8.h               | 4 ++++
>>> >   ell/util.h               | 4 ++++
>>> >   unit/test-dbus-message.c | 3 ++-
>>> >   8 files changed, 24 insertions(+), 3 deletions(-)
>>> >
>>>
>>> All applied, thanks.
>>>
>>> Regards,
>>> -Denis
>>> _______________________________________________
>>> ell mailing list
>>> ell(a)lists.01.org
>>> https://lists.01.org/mailman/listinfo/ell
>>>
>>

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 3243 bytes --]

Re: [PATCH 0/5] Support C89 ELL builds

[Denis Kenzior]

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

Hi Michał,

On 03/05/2019 12:57 AM, Michał Lowas-Rzechonek wrote:
> Yeah, I get that! My question was mostly to Denis, about the overall policy.
> 

Please, no top posting on this mailing list :)

So my initial response would be in a form of a bewildered question: "Why 
are we even talking about this?".  The reason I ask is that we follow 
the Linux kernel coding style, which basically disallows most of the 
'interesting' C99 features.

But since we are talking about this, let me formulate the official 
policy.  We follow the same process as described in: 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/programming-language.rst

So roughly speaking what this means is that the code should compile with 
-std=gnu89.  C89 compliance is not an official policy or goal.

Regards,
-Denis

Re: [PATCH 0/5] Support C89 ELL builds

[=?unknown-8bit?q?Micha=C5=82?= Lowas-Rzechonek]

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

Hi Denis,

On 03/05, Denis Kenzior wrote:
> Please, no top posting on this mailing list :)
Sorry! Most MUAs nowadays default to top posting, my bad for not paying
attention.

> So my initial response would be in a form of a bewildered question: "Why are
> we even talking about this?".  The reason I ask is that we follow the
> Linux kernel coding style, which basically disallows most of the
> 'interesting' C99 features.
Didn't mean to offend. Since I submitted a patch and I'm probably going
to submit a few more, I wanted to know what to expect. I read the coding
style, but ELL doesn't mention -std=gnu89. I know it's probably
"obvious" to you guys, since you have a kernel background, but many
other projects have substantially different policies.

> But since we are talking about this, let me formulate the official policy.
> We follow the same process as described in:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/programming-language.rst
>
> So roughly speaking what this means is that the code should compile with
> -std=gnu89.  C89 compliance is not an official policy or goal.
Fair enough, thank you for clarification!

It might be worth to mention this in doc/coding-style, so a next
clueless person doesn't repeat silly questions ;)

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND

Re: [PATCH 0/5] Support C89 ELL builds

[Denis Kenzior]

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

Hi Michał,

On 03/05/2019 12:56 PM, Michał Lowas-Rzechonek wrote:
> Hi Denis,
> 
> On 03/05, Denis Kenzior wrote:
>> Please, no top posting on this mailing list :)
> Sorry! Most MUAs nowadays default to top posting, my bad for not paying
> attention.

No worries.

> 
>> So my initial response would be in a form of a bewildered question: "Why are
>> we even talking about this?".  The reason I ask is that we follow the
>> Linux kernel coding style, which basically disallows most of the
>> 'interesting' C99 features.
> Didn't mean to offend. Since I submitted a patch and I'm probably going
> to submit a few more, I wanted to know what to expect. I read the coding
> style, but ELL doesn't mention -std=gnu89. I know it's probably
> "obvious" to you guys, since you have a kernel background, but many
> other projects have substantially different policies.

No, not at all, no offense taken.  I was just incredulous that this 
topic came up at all.  The coding style does mention that we use the 
Linux Kernel coding style, and if you study that one it becomes pretty 
clear that all the 'fun' features of C99 aren't allowed.  Also, if you 
violate the coding standard, then I will also likely mention it during 
the initial review ;)

> 
>> But since we are talking about this, let me formulate the official policy.
>> We follow the same process as described in:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/programming-language.rst
>>
>> So roughly speaking what this means is that the code should compile with
>> -std=gnu89.  C89 compliance is not an official policy or goal.
> Fair enough, thank you for clarification!
> 
> It might be worth to mention this in doc/coding-style, so a next
> clueless person doesn't repeat silly questions ;)
> 

In this country they say: 'There are no silly questions'.  Anyhow, point 
taken and I've added a reference to the Linux Kernel 'programming 
language process' document in commit: 
4c220dca8c7b13774a7af5cd7a3ed06ef4d88a73

Hopefully that makes things clearer.

Regards,
-Denis