[kmod] shared/util.c: assert_cc() can only be used inside functions
diff mbox

Message ID 1496502202-9832-1-git-send-email-thomas.petazzoni@free-electrons.com
State New
Headers show

Commit Message

Thomas Petazzoni June 3, 2017, 3:03 p.m. UTC
shared/macro.h has two versions of assert_cc, one that uses gcc
_Static_assert(), which requires recent enough gcc versions, and one
that uses a fake array to trigger a build error. The latter can only
work inside functions, so assert_cc() should only be used inside
functions.

Fixes the following build failure when building kmod with old gcc
versions such as gcc 4.3.x:

shared/util.c:52: error: expected identifier or '(' before 'do'
shared/util.c:52: error: expected identifier or '(' before 'while'

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 shared/util.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Lucas De Marchi June 5, 2017, 8:04 a.m. UTC | #1
On Sat, Jun 3, 2017 at 8:03 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> shared/macro.h has two versions of assert_cc, one that uses gcc
> _Static_assert(), which requires recent enough gcc versions, and one
> that uses a fake array to trigger a build error. The latter can only
> work inside functions, so assert_cc() should only be used inside
> functions.
>
> Fixes the following build failure when building kmod with old gcc
> versions such as gcc 4.3.x:
>
> shared/util.c:52: error: expected identifier or '(' before 'do'
> shared/util.c:52: error: expected identifier or '(' before 'while'
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---

The changes look simple, but going forward I'd like to understand
this. kmod requires C11 that contains _Static_assert().

Is there a compelling reason to support a compiler that old? GCC 4.3.0
has been released 9 years ago.

Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Petazzoni June 5, 2017, 8:22 a.m. UTC | #2
Hello Lucas,

Thanks for your feedback!

On Mon, 5 Jun 2017 01:04:46 -0700, Lucas De Marchi wrote:

> The changes look simple, but going forward I'd like to understand
> this. kmod requires C11 that contains _Static_assert().

kmod seems to build fine with a compiler that isn't C11.

> Is there a compelling reason to support a compiler that old? GCC 4.3.0
> has been released 9 years ago.

As you know, I'm contributing to Buildroot, an embedded Linux build
system, that allows to build from scratch lightweight, configurable
Linux systems through cross-compilation.

Buildroot is widely used in enterprise contexts, where sometimes very
old Linux distributions are used on build servers. As an example, we
even have to look at the version of the 'tar' utility available on the
host, and build our own if it's too old, because some old RHEL distros
have a tar version that is bogus.

In order to make sure our users in this situation don't face problems,
we run test builds on old distros, and one of my automated build
machine has a Debian old enough to still be based on gcc 4.3.

Of course, for the target compiler, we use something more modern (we
recently switched to gcc 6.x as our default compiler version). But the
host compiler is whatever is provided on the user's system, which we
don't control.

Best regards,

Thomas Petazzoni
Lucas De Marchi June 5, 2017, 5:06 p.m. UTC | #3
Hi,

On Mon, Jun 5, 2017 at 1:22 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello Lucas,
>
> Thanks for your feedback!
>
> On Mon, 5 Jun 2017 01:04:46 -0700, Lucas De Marchi wrote:
>
>> The changes look simple, but going forward I'd like to understand
>> this. kmod requires C11 that contains _Static_assert().
>
> kmod seems to build fine with a compiler that isn't C11.

This is pure luck :) - C11 is a requirement. We don't enforce it in
the configure because there wasn't a way to do so with autoconf macros
without creating our own back in the time when C11 started to be a
requirement.


>> Is there a compelling reason to support a compiler that old? GCC 4.3.0
>> has been released 9 years ago.
>
> As you know, I'm contributing to Buildroot, an embedded Linux build
> system, that allows to build from scratch lightweight, configurable
> Linux systems through cross-compilation.

Yep, I think I even may have 1 or 2 patches to buildroot :)

> Buildroot is widely used in enterprise contexts, where sometimes very
> old Linux distributions are used on build servers. As an example, we
> even have to look at the version of the 'tar' utility available on the
> host, and build our own if it's too old, because some old RHEL distros
> have a tar version that is bogus.
>
> In order to make sure our users in this situation don't face problems,
> we run test builds on old distros, and one of my automated build
> machine has a Debian old enough to still be based on gcc 4.3.

I don't want to personally maintain compatibility with that old
compiler. I'll apply this, but it may break again in future.  If
breakage is too often, then I'll ask for you to maintain those patches
as downstream patches, ok?

thanks
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Petazzoni June 6, 2017, 7:16 a.m. UTC | #4
Hello,

On Mon, 5 Jun 2017 10:06:30 -0700, Lucas De Marchi wrote:

> > As you know, I'm contributing to Buildroot, an embedded Linux build
> > system, that allows to build from scratch lightweight, configurable
> > Linux systems through cross-compilation.  
> 
> Yep, I think I even may have 1 or 2 patches to buildroot :)

Yes, I know you're aware what Buildroot is :-) It was more an
explanation for the rest of the readers of this mailing list.

> > Buildroot is widely used in enterprise contexts, where sometimes very
> > old Linux distributions are used on build servers. As an example, we
> > even have to look at the version of the 'tar' utility available on the
> > host, and build our own if it's too old, because some old RHEL distros
> > have a tar version that is bogus.
> >
> > In order to make sure our users in this situation don't face problems,
> > we run test builds on old distros, and one of my automated build
> > machine has a Debian old enough to still be based on gcc 4.3.  
> 
> I don't want to personally maintain compatibility with that old
> compiler. I'll apply this, but it may break again in future.  If
> breakage is too often, then I'll ask for you to maintain those patches
> as downstream patches, ok?

We generally try to avoid maintaining downstream patches. So when that
happens, we'll probably just add the requirements that gcc version XYZ
is needed. And ask unhappy people to come complain to upstream
developers :-)

Thanks!

Thomas Petazzoni

Patch
diff mbox

diff --git a/shared/util.c b/shared/util.c
index 9de080a..fd2028d 100644
--- a/shared/util.c
+++ b/shared/util.c
@@ -49,8 +49,6 @@  static const struct kmod_ext {
 	{ }
 };
 
-assert_cc(EAGAIN == EWOULDBLOCK);
-
 /* string handling functions and memory allocations                         */
 /* ************************************************************************ */
 
@@ -201,6 +199,8 @@  ssize_t read_str_safe(int fd, char *buf, size_t buflen)
 	size_t todo = buflen - 1;
 	size_t done = 0;
 
+	assert_cc(EAGAIN == EWOULDBLOCK);
+
 	do {
 		ssize_t r = read(fd, buf + done, todo);
 
@@ -226,6 +226,8 @@  ssize_t write_str_safe(int fd, const char *buf, size_t buflen)
 	size_t todo = buflen;
 	size_t done = 0;
 
+	assert_cc(EAGAIN == EWOULDBLOCK);
+
 	do {
 		ssize_t r = write(fd, buf + done, todo);