From: Mikko Rapeli <hidden> Date: 2017-08-06 17:24:07
linux/time.h conflicts with user space header time.h. Try to be compatible
with both.
Fixes userspace compilation error:
error: array type has incomplete element type
struct timespec ts[3];
Signed-off-by: Mikko Rapeli <redacted>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Soheil Hassas Yeganeh <redacted>
Cc: netdev@vger.kernel.org
---
include/uapi/linux/errqueue.h | 6 ++++++
1 file changed, 6 insertions(+)
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2017-08-06 20:23:59
On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli [off-list ref] wrote:
quoted hunk
linux/time.h conflicts with user space header time.h. Try to be compatible
with both.
Fixes userspace compilation error:
error: array type has incomplete element type
struct timespec ts[3];
Signed-off-by: Mikko Rapeli <redacted>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Soheil Hassas Yeganeh <redacted>
Cc: netdev@vger.kernel.org
---
include/uapi/linux/errqueue.h | 6 ++++++
1 file changed, 6 insertions(+)
This will break applications that include <linux/time.h> manually.
I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where <linux/time.h> is included after
<time.h>.
https://lkml.org/lkml/2016/9/12/872
The inverse will require changes to the libc header to avoid redefining
symbols already defined by <linux/time.h>
The second patch in that 2-patch set included <linux/time.h>
unconditionally after the fix. This broke builds that also included
<time.h> in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com> Date: 2017-08-06 20:27:34
On Sun, Aug 6, 2017 at 4:23 PM, Willem de Bruijn
[off-list ref] wrote:
On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli [off-list ref] wrote:
quoted
linux/time.h conflicts with user space header time.h. Try to be compatible
with both.
Fixes userspace compilation error:
error: array type has incomplete element type
struct timespec ts[3];
Signed-off-by: Mikko Rapeli <redacted>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Soheil Hassas Yeganeh <redacted>
Cc: netdev@vger.kernel.org
---
include/uapi/linux/errqueue.h | 6 ++++++
1 file changed, 6 insertions(+)
From: Mikko Rapeli <hidden> Date: 2017-08-06 20:56:59
On Sun, Aug 06, 2017 at 04:23:16PM -0400, Willem de Bruijn wrote:
On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli [off-list ref] wrote:
quoted
linux/time.h conflicts with user space header time.h. Try to be compatible
with both.
Fixes userspace compilation error:
error: array type has incomplete element type
struct timespec ts[3];
Signed-off-by: Mikko Rapeli <redacted>
Cc: Willem de Bruijn <redacted>
Cc: Soheil Hassas Yeganeh <redacted>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
include/uapi/linux/errqueue.h | 6 ++++++
1 file changed, 6 insertions(+)
This will break applications that include <linux/time.h> manually.
I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where <linux/time.h> is included after
<time.h>.
https://lkml.org/lkml/2016/9/12/872
The inverse will require changes to the libc header to avoid redefining
symbols already defined by <linux/time.h>
The second patch in that 2-patch set included <linux/time.h>
unconditionally after the fix. This broke builds that also included
<time.h> in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.
I wasn't aware of your change, but I was about to send this to fix the
case when glibc <time.h> is included before <linux/time.h>:
https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
but you also ran into problems where <linux/time.h> is included before
<time.h> which need fixes in libc header side.
So how to proceed with these?
I don't like leaving a few dozen non-compiling header files into uapi.
-Mikko
From: Mikko Rapeli <hidden> Date: 2017-08-06 20:58:29
On Sun, Aug 06, 2017 at 04:26:50PM -0400, Willem de Bruijn wrote:
On Sun, Aug 6, 2017 at 4:23 PM, Willem de Bruijn
[off-list ref] wrote:
quoted
On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli [off-list ref] wrote:
quoted
linux/time.h conflicts with user space header time.h. Try to be compatible
with both.
Fixes userspace compilation error:
error: array type has incomplete element type
struct timespec ts[3];
Signed-off-by: Mikko Rapeli <redacted>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Soheil Hassas Yeganeh <redacted>
Cc: netdev@vger.kernel.org
---
include/uapi/linux/errqueue.h | 6 ++++++
1 file changed, 6 insertions(+)
This will break applications that include <linux/time.h> manually.
I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where <linux/time.h> is included after
<time.h>.
https://lkml.org/lkml/2016/9/12/872
The inverse will require changes to the libc header to avoid redefining
symbols already defined by <linux/time.h>
The second patch in that 2-patch set included <linux/time.h>
unconditionally after the fix. This broke builds that also included
<time.h> in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.
There are a few differences between the two. Including <time.h> does not
unconditionally define all the symbols. Some are conditional on additional
state, such as __timespec_defined.
but you also ran into problems where <linux/time.h> is included before
<time.h> which need fixes in libc header side.
So how to proceed with these?
The libc-compat change is a good fix that can be submitted on its own.
I don't like leaving a few dozen non-compiling header files into uapi.
I agree, but I do not see a simple solution.
Unless libc has the analogous change, including either <time.h> or
<linux/time.h> in userspace can unfortunately cause breakage.
The added include if __KERNEL__ is defined should be safe, though.
This will break applications that include <linux/time.h> manually.
I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where <linux/time.h> is included after
<time.h>.
https://lkml.org/lkml/2016/9/12/872
The inverse will require changes to the libc header to avoid redefining
symbols already defined by <linux/time.h>
The second patch in that 2-patch set included <linux/time.h>
unconditionally after the fix. This broke builds that also included
<time.h> in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.
There are a few differences between the two. Including <time.h> does not
unconditionally define all the symbols. Some are conditional on additional
state, such as __timespec_defined.
Yep, your patch seems better for libc-compat.h. Could you send it again?
quoted
but you also ran into problems where <linux/time.h> is included before
<time.h> which need fixes in libc header side.
So how to proceed with these?
The libc-compat change is a good fix that can be submitted on its own.
Yes, please do so.
quoted
I don't like leaving a few dozen non-compiling header files into uapi.
I agree, but I do not see a simple solution.
Unless libc has the analogous change, including either <time.h> or
<linux/time.h> in userspace can unfortunately cause breakage.
The added include if __KERNEL__ is defined should be safe, though.
Yes, for the kernel side, but your libc-compat change would nice for
userspace, where something will break for sure, but providing source
API compatibility is sometimes impossible.
To summarize, this change from me, and your libc-compat.c for time.h, or?
-Mikko
This will break applications that include <linux/time.h> manually.
I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where <linux/time.h> is included after
<time.h>.
https://lkml.org/lkml/2016/9/12/872
The inverse will require changes to the libc header to avoid redefining
symbols already defined by <linux/time.h>
The second patch in that 2-patch set included <linux/time.h>
unconditionally after the fix. This broke builds that also included
<time.h> in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.
There are a few differences between the two. Including <time.h> does not
unconditionally define all the symbols. Some are conditional on additional
state, such as __timespec_defined.
Yep, your patch seems better for libc-compat.h. Could you send it again?
Okay. Or feel free to include it in the patchset if that helps resolve
dependencies.
quoted
quoted
I don't like leaving a few dozen non-compiling header files into uapi.
I agree, but I do not see a simple solution.
Unless libc has the analogous change, including either <time.h> or
<linux/time.h> in userspace can unfortunately cause breakage.
The added include if __KERNEL__ is defined should be safe, though.
Yes, for the kernel side, but your libc-compat change would nice for
userspace, where something will break for sure, but providing source
API compatibility is sometimes impossible.
To summarize, this change from me, and your libc-compat.c for time.h, or?
I'm still afraid that this patch as is will break builds that include
<linux/time.h> first.
This will break applications that include <linux/time.h> manually.
I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where <linux/time.h> is included after
<time.h>.
https://lkml.org/lkml/2016/9/12/872
The inverse will require changes to the libc header to avoid redefining
symbols already defined by <linux/time.h>
The second patch in that 2-patch set included <linux/time.h>
unconditionally after the fix. This broke builds that also included
<time.h> in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.
There are a few differences between the two. Including <time.h> does not
unconditionally define all the symbols. Some are conditional on additional
state, such as __timespec_defined.
Yep, your patch seems better for libc-compat.h. Could you send it again?
Okay. Or feel free to include it in the patchset if that helps resolve
dependencies.
If you don't have the time, I will send tomorrow a new version of this
patch which fixes the commit topic and before that your libc-compat.h change
so both could be applied together.
Feel free to be faster :)
quoted
quoted
quoted
I don't like leaving a few dozen non-compiling header files into uapi.
I agree, but I do not see a simple solution.
Unless libc has the analogous change, including either <time.h> or
<linux/time.h> in userspace can unfortunately cause breakage.
The added include if __KERNEL__ is defined should be safe, though.
Yes, for the kernel side, but your libc-compat change would nice for
userspace, where something will break for sure, but providing source
API compatibility is sometimes impossible.
To summarize, this change from me, and your libc-compat.c for time.h, or?
I'm still afraid that this patch as is will break builds that include
<linux/time.h> first.
I agree, but I also want uapi headers to cleanly compile. I know this might
break stuff on userspace side which rely on these broken header file
dependencies, but if the fix to just re-order include
statements I'm fine with it, also when the complaints hit my inbox.
If I had the CPU time, memory and disk space, I'd do a full yocto distro
build to see how badly userspace could break but I don't at home.
-Mikko
This will break applications that include <linux/time.h> manually.
I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where <linux/time.h> is included after
<time.h>.
https://lkml.org/lkml/2016/9/12/872
The inverse will require changes to the libc header to avoid redefining
symbols already defined by <linux/time.h>
The second patch in that 2-patch set included <linux/time.h>
unconditionally after the fix. This broke builds that also included
<time.h> in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.
There are a few differences between the two. Including <time.h> does not
unconditionally define all the symbols. Some are conditional on additional
state, such as __timespec_defined.
Yep, your patch seems better for libc-compat.h. Could you send it again?
Okay. Or feel free to include it in the patchset if that helps resolve
dependencies.
If you don't have the time, I will send tomorrow a new version of this
patch which fixes the commit topic and before that your libc-compat.h change
so both could be applied together.