Linux-Modules Archive on lore.kernel.org
 help / Atom feed
* [PATCH kmod] shared/util.c: assert_cc() can only be used inside functions
@ 2017-06-03 15:03 Thomas Petazzoni
  2017-06-05  8:04 ` Lucas De Marchi
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2017-06-03 15:03 UTC (permalink / raw)
  To: linux-modules; +Cc: Thomas Petazzoni

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(-)

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);
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH kmod] shared/util.c: assert_cc() can only be used inside functions
  2017-06-03 15:03 [PATCH kmod] shared/util.c: assert_cc() can only be used inside functions Thomas Petazzoni
@ 2017-06-05  8:04 ` Lucas De Marchi
  2017-06-05  8:22   ` Thomas Petazzoni
  0 siblings, 1 reply; 5+ messages in thread
From: Lucas De Marchi @ 2017-06-05  8:04 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-modules

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH kmod] shared/util.c: assert_cc() can only be used inside functions
  2017-06-05  8:04 ` Lucas De Marchi
@ 2017-06-05  8:22   ` Thomas Petazzoni
  2017-06-05 17:06     ` Lucas De Marchi
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2017-06-05  8:22 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH kmod] shared/util.c: assert_cc() can only be used inside functions
  2017-06-05  8:22   ` Thomas Petazzoni
@ 2017-06-05 17:06     ` Lucas De Marchi
  2017-06-06  7:16       ` Thomas Petazzoni
  0 siblings, 1 reply; 5+ messages in thread
From: Lucas De Marchi @ 2017-06-05 17:06 UTC (permalink / raw)
  To: Thomas Petazzoni; +Cc: linux-modules

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH kmod] shared/util.c: assert_cc() can only be used inside functions
  2017-06-05 17:06     ` Lucas De Marchi
@ 2017-06-06  7:16       ` Thomas Petazzoni
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Petazzoni @ 2017-06-06  7:16 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules

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
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-03 15:03 [PATCH kmod] shared/util.c: assert_cc() can only be used inside functions Thomas Petazzoni
2017-06-05  8:04 ` Lucas De Marchi
2017-06-05  8:22   ` Thomas Petazzoni
2017-06-05 17:06     ` Lucas De Marchi
2017-06-06  7:16       ` Thomas Petazzoni

Linux-Modules Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-modules/0 linux-modules/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-modules linux-modules/ https://lore.kernel.org/linux-modules \
		linux-modules@vger.kernel.org linux-modules@archiver.kernel.org
	public-inbox-index linux-modules


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-modules


AGPL code for this site: git clone https://public-inbox.org/ public-inbox