6:42 p.m.
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
6:42 p.m.
New subject: [PATCH 1/5] ell: Use __inline__ instead of inline for C89.
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
6:42 p.m.
New subject: [PATCH 2/5] ell: Do not use C99 '//' single line comment.
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
6:42 p.m.
New subject: [PATCH 3/5] settings: Explicitly enable strto{u}ll() for C89.
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
6:42 p.m.
New subject: [PATCH 4/5] string: Use __va_copy() for pre-C99 builds.
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
6:42 p.m.
New subject: [PATCH 5/5] unit: Remove C99 loop variable declaration.
---
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
7:19 p.m.
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
9:15 p.m.
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(a)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
9:58 p.m.
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(a)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
>
6:57 a.m.
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(a)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(a)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
>>
>
5:43 p.m.
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/D...
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
6:56 p.m.
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/D...
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(a)silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND
8:10 p.m.
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/D...
>
> 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
964
days inactive
965
days old
12 comments
4 participants
participants (4)
-
Denis Kenzior -
Michał Lowas-Rzechonek -
Ossama Othman -
Othman, Ossama