Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 5:52 PM, Mikko Rapeli  wrote:
> On Sun, Aug 06, 2017 at 05:42:13PM -0400, Willem de Bruijn wrote:
>> On Sun, Aug 6, 2017 at 5:33 PM, Mikko Rapeli  wrote:
>> > On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
>> >> >> > +#ifdef __KERNEL__
>> >> >> > +#include 
>> >> >> > +#else
>> >> >> > +#include 
>> >> >> > +#endif /* __KERNEL__ */
>> >> >>
>> >> >> This will break applications that include  manually.
>> >> >> I previously sent a patch to use libc-compat to make compilation 
>> >> >> succeed
>> >> >> when both are included in the case where  is included 
>> >> >> after
>> >> >> .
>> >> >>
>> >> >>   https://lkml.org/lkml/2016/9/12/872
>> >> >>
>> >> >> The inverse will require changes to the libc header to avoid redefining
>> >> >> symbols already defined by 
>> >> >>
>> >> >> The second patch in that 2-patch set included 
>> >> >> unconditionally after the fix. This broke builds that also included
>> >> >>  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  is included before :
>> >> >
>> >> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
>> >>
>> >> There are a few differences between the two. Including  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.

Please do. Thanks!

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 5:52 PM, Mikko Rapeli  wrote:
> On Sun, Aug 06, 2017 at 05:42:13PM -0400, Willem de Bruijn wrote:
>> On Sun, Aug 6, 2017 at 5:33 PM, Mikko Rapeli  wrote:
>> > On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
>> >> >> > +#ifdef __KERNEL__
>> >> >> > +#include 
>> >> >> > +#else
>> >> >> > +#include 
>> >> >> > +#endif /* __KERNEL__ */
>> >> >>
>> >> >> This will break applications that include  manually.
>> >> >> I previously sent a patch to use libc-compat to make compilation 
>> >> >> succeed
>> >> >> when both are included in the case where  is included 
>> >> >> after
>> >> >> .
>> >> >>
>> >> >>   https://lkml.org/lkml/2016/9/12/872
>> >> >>
>> >> >> The inverse will require changes to the libc header to avoid redefining
>> >> >> symbols already defined by 
>> >> >>
>> >> >> The second patch in that 2-patch set included 
>> >> >> unconditionally after the fix. This broke builds that also included
>> >> >>  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  is included before :
>> >> >
>> >> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
>> >>
>> >> There are a few differences between the two. Including  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.

Please do. Thanks!

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

On Sun, Aug 06, 2017 at 05:42:13PM -0400, Willem de Bruijn wrote:
> On Sun, Aug 6, 2017 at 5:33 PM, Mikko Rapeli  wrote:
> > On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
> >> >> > +#ifdef __KERNEL__
> >> >> > +#include 
> >> >> > +#else
> >> >> > +#include 
> >> >> > +#endif /* __KERNEL__ */
> >> >>
> >> >> This will break applications that include  manually.
> >> >> I previously sent a patch to use libc-compat to make compilation succeed
> >> >> when both are included in the case where  is included 
> >> >> after
> >> >> .
> >> >>
> >> >>   https://lkml.org/lkml/2016/9/12/872
> >> >>
> >> >> The inverse will require changes to the libc header to avoid redefining
> >> >> symbols already defined by 
> >> >>
> >> >> The second patch in that 2-patch set included 
> >> >> unconditionally after the fix. This broke builds that also included
> >> >>  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  is included before :
> >> >
> >> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
> >>
> >> There are a few differences between the two. Including  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 :)

> >> > 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  or
> >>  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
>  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

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

On Sun, Aug 06, 2017 at 05:42:13PM -0400, Willem de Bruijn wrote:
> On Sun, Aug 6, 2017 at 5:33 PM, Mikko Rapeli  wrote:
> > On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
> >> >> > +#ifdef __KERNEL__
> >> >> > +#include 
> >> >> > +#else
> >> >> > +#include 
> >> >> > +#endif /* __KERNEL__ */
> >> >>
> >> >> This will break applications that include  manually.
> >> >> I previously sent a patch to use libc-compat to make compilation succeed
> >> >> when both are included in the case where  is included 
> >> >> after
> >> >> .
> >> >>
> >> >>   https://lkml.org/lkml/2016/9/12/872
> >> >>
> >> >> The inverse will require changes to the libc header to avoid redefining
> >> >> symbols already defined by 
> >> >>
> >> >> The second patch in that 2-patch set included 
> >> >> unconditionally after the fix. This broke builds that also included
> >> >>  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  is included before :
> >> >
> >> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
> >>
> >> There are a few differences between the two. Including  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 :)

> >> > 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  or
> >>  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
>  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

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 5:33 PM, Mikko Rapeli  wrote:
> On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
>> >> > +#ifdef __KERNEL__
>> >> > +#include 
>> >> > +#else
>> >> > +#include 
>> >> > +#endif /* __KERNEL__ */
>> >>
>> >> This will break applications that include  manually.
>> >> I previously sent a patch to use libc-compat to make compilation succeed
>> >> when both are included in the case where  is included after
>> >> .
>> >>
>> >>   https://lkml.org/lkml/2016/9/12/872
>> >>
>> >> The inverse will require changes to the libc header to avoid redefining
>> >> symbols already defined by 
>> >>
>> >> The second patch in that 2-patch set included 
>> >> unconditionally after the fix. This broke builds that also included
>> >>  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  is included before :
>> >
>> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
>>
>> There are a few differences between the two. Including  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.

>> > 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  or
>>  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
 first.

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 5:33 PM, Mikko Rapeli  wrote:
> On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
>> >> > +#ifdef __KERNEL__
>> >> > +#include 
>> >> > +#else
>> >> > +#include 
>> >> > +#endif /* __KERNEL__ */
>> >>
>> >> This will break applications that include  manually.
>> >> I previously sent a patch to use libc-compat to make compilation succeed
>> >> when both are included in the case where  is included after
>> >> .
>> >>
>> >>   https://lkml.org/lkml/2016/9/12/872
>> >>
>> >> The inverse will require changes to the libc header to avoid redefining
>> >> symbols already defined by 
>> >>
>> >> The second patch in that 2-patch set included 
>> >> unconditionally after the fix. This broke builds that also included
>> >>  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  is included before :
>> >
>> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
>>
>> There are a few differences between the two. Including  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.

>> > 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  or
>>  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
 first.

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
> >> > +#ifdef __KERNEL__
> >> > +#include 
> >> > +#else
> >> > +#include 
> >> > +#endif /* __KERNEL__ */
> >>
> >> This will break applications that include  manually.
> >> I previously sent a patch to use libc-compat to make compilation succeed
> >> when both are included in the case where  is included after
> >> .
> >>
> >>   https://lkml.org/lkml/2016/9/12/872
> >>
> >> The inverse will require changes to the libc header to avoid redefining
> >> symbols already defined by 
> >>
> >> The second patch in that 2-patch set included 
> >> unconditionally after the fix. This broke builds that also included
> >>  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  is included before :
> >
> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
> 
> There are a few differences between the two. Including  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?

> > but you also ran into problems where  is included before
> >  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.

> > 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  or
>  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

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

On Sun, Aug 06, 2017 at 05:24:20PM -0400, Willem de Bruijn wrote:
> >> > +#ifdef __KERNEL__
> >> > +#include 
> >> > +#else
> >> > +#include 
> >> > +#endif /* __KERNEL__ */
> >>
> >> This will break applications that include  manually.
> >> I previously sent a patch to use libc-compat to make compilation succeed
> >> when both are included in the case where  is included after
> >> .
> >>
> >>   https://lkml.org/lkml/2016/9/12/872
> >>
> >> The inverse will require changes to the libc header to avoid redefining
> >> symbols already defined by 
> >>
> >> The second patch in that 2-patch set included 
> >> unconditionally after the fix. This broke builds that also included
> >>  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  is included before :
> >
> > https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66
> 
> There are a few differences between the two. Including  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?

> > but you also ran into problems where  is included before
> >  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.

> > 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  or
>  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

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

>> > +#ifdef __KERNEL__
>> > +#include 
>> > +#else
>> > +#include 
>> > +#endif /* __KERNEL__ */
>>
>> This will break applications that include  manually.
>> I previously sent a patch to use libc-compat to make compilation succeed
>> when both are included in the case where  is included after
>> .
>>
>>   https://lkml.org/lkml/2016/9/12/872
>>
>> The inverse will require changes to the libc header to avoid redefining
>> symbols already defined by 
>>
>> The second patch in that 2-patch set included 
>> unconditionally after the fix. This broke builds that also included
>>  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  is included before :
>
> https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66

There are a few differences between the two. Including  does not
unconditionally define all the symbols. Some are conditional on additional
state, such as __timespec_defined.

> but you also ran into problems where  is included before
>  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  or
 in userspace can unfortunately cause breakage.

The added include if __KERNEL__ is defined should be safe, though.

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

>> > +#ifdef __KERNEL__
>> > +#include 
>> > +#else
>> > +#include 
>> > +#endif /* __KERNEL__ */
>>
>> This will break applications that include  manually.
>> I previously sent a patch to use libc-compat to make compilation succeed
>> when both are included in the case where  is included after
>> .
>>
>>   https://lkml.org/lkml/2016/9/12/872
>>
>> The inverse will require changes to the libc header to avoid redefining
>> symbols already defined by 
>>
>> The second patch in that 2-patch set included 
>> unconditionally after the fix. This broke builds that also included
>>  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  is included before :
>
> https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66

There are a few differences between the two. Including  does not
unconditionally define all the symbols. Some are conditional on additional
state, such as __timespec_defined.

> but you also ran into problems where  is included before
>  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  or
 in userspace can unfortunately cause breakage.

The added include if __KERNEL__ is defined should be safe, though.

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

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
>  wrote:
> > On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli  wrote:
> >> 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 
> >> Cc: Willem de Bruijn 
> >> Cc: Soheil Hassas Yeganeh 
> >> Cc: net...@vger.kernel.org
> >> ---
> >>  include/uapi/linux/errqueue.h | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
> >> index 07bdce1f444a..b310b2c6d94f 100644
> >> --- a/include/uapi/linux/errqueue.h
> >> +++ b/include/uapi/linux/errqueue.h
> >> @@ -3,6 +3,12 @@
> >>
> >>  #include 
> >>
> >> +#ifdef __KERNEL__
> >> +#include 
> >> +#else
> >> +#include 
> >> +#endif /* __KERNEL__ */
> >
> > This will break applications that include  manually.
> 
> Also, the patch title reads "include  in user space",
> but it includes  in that environment.

Oops, missed while squashing some commits. Will fix. Thanks!

-Mikko

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

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
>  wrote:
> > On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli  wrote:
> >> 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 
> >> Cc: Willem de Bruijn 
> >> Cc: Soheil Hassas Yeganeh 
> >> Cc: net...@vger.kernel.org
> >> ---
> >>  include/uapi/linux/errqueue.h | 6 ++
> >>  1 file changed, 6 insertions(+)
> >>
> >> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
> >> index 07bdce1f444a..b310b2c6d94f 100644
> >> --- a/include/uapi/linux/errqueue.h
> >> +++ b/include/uapi/linux/errqueue.h
> >> @@ -3,6 +3,12 @@
> >>
> >>  #include 
> >>
> >> +#ifdef __KERNEL__
> >> +#include 
> >> +#else
> >> +#include 
> >> +#endif /* __KERNEL__ */
> >
> > This will break applications that include  manually.
> 
> Also, the patch title reads "include  in user space",
> but it includes  in that environment.

Oops, missed while squashing some commits. Will fix. Thanks!

-Mikko

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

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  wrote:
> > 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 
> > Cc: Willem de Bruijn 
> > Cc: Soheil Hassas Yeganeh 
> > Cc: net...@vger.kernel.org
> > ---
> >  include/uapi/linux/errqueue.h | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
> > index 07bdce1f444a..b310b2c6d94f 100644
> > --- a/include/uapi/linux/errqueue.h
> > +++ b/include/uapi/linux/errqueue.h
> > @@ -3,6 +3,12 @@
> >
> >  #include 
> >
> > +#ifdef __KERNEL__
> > +#include 
> > +#else
> > +#include 
> > +#endif /* __KERNEL__ */
> 
> This will break applications that include  manually.
> I previously sent a patch to use libc-compat to make compilation succeed
> when both are included in the case where  is included after
> .
> 
>   https://lkml.org/lkml/2016/9/12/872
> 
> The inverse will require changes to the libc header to avoid redefining
> symbols already defined by 
> 
> The second patch in that 2-patch set included 
> unconditionally after the fix. This broke builds that also included
>  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  is included before :

https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66

but you also ran into problems where  is included before
 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

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

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  wrote:
> > 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 
> > Cc: Willem de Bruijn 
> > Cc: Soheil Hassas Yeganeh 
> > Cc: net...@vger.kernel.org
> > ---
> >  include/uapi/linux/errqueue.h | 6 ++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
> > index 07bdce1f444a..b310b2c6d94f 100644
> > --- a/include/uapi/linux/errqueue.h
> > +++ b/include/uapi/linux/errqueue.h
> > @@ -3,6 +3,12 @@
> >
> >  #include 
> >
> > +#ifdef __KERNEL__
> > +#include 
> > +#else
> > +#include 
> > +#endif /* __KERNEL__ */
> 
> This will break applications that include  manually.
> I previously sent a patch to use libc-compat to make compilation succeed
> when both are included in the case where  is included after
> .
> 
>   https://lkml.org/lkml/2016/9/12/872
> 
> The inverse will require changes to the libc header to avoid redefining
> symbols already defined by 
> 
> The second patch in that 2-patch set included 
> unconditionally after the fix. This broke builds that also included
>  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  is included before :

https://github.com/mcfrisk/linux/commit/f3952a27b8a21c6478d26e6246055383483f6a66

but you also ran into problems where  is included before
 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

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 4:23 PM, Willem de Bruijn
 wrote:
> On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli  wrote:
>> 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 
>> Cc: Willem de Bruijn 
>> Cc: Soheil Hassas Yeganeh 
>> Cc: net...@vger.kernel.org
>> ---
>>  include/uapi/linux/errqueue.h | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
>> index 07bdce1f444a..b310b2c6d94f 100644
>> --- a/include/uapi/linux/errqueue.h
>> +++ b/include/uapi/linux/errqueue.h
>> @@ -3,6 +3,12 @@
>>
>>  #include 
>>
>> +#ifdef __KERNEL__
>> +#include 
>> +#else
>> +#include 
>> +#endif /* __KERNEL__ */
>
> This will break applications that include  manually.

Also, the patch title reads "include  in user space",
but it includes  in that environment.

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 4:23 PM, Willem de Bruijn
 wrote:
> On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli  wrote:
>> 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 
>> Cc: Willem de Bruijn 
>> Cc: Soheil Hassas Yeganeh 
>> Cc: net...@vger.kernel.org
>> ---
>>  include/uapi/linux/errqueue.h | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
>> index 07bdce1f444a..b310b2c6d94f 100644
>> --- a/include/uapi/linux/errqueue.h
>> +++ b/include/uapi/linux/errqueue.h
>> @@ -3,6 +3,12 @@
>>
>>  #include 
>>
>> +#ifdef __KERNEL__
>> +#include 
>> +#else
>> +#include 
>> +#endif /* __KERNEL__ */
>
> This will break applications that include  manually.

Also, the patch title reads "include  in user space",
but it includes  in that environment.

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli  wrote:
> 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 
> Cc: Willem de Bruijn 
> Cc: Soheil Hassas Yeganeh 
> Cc: net...@vger.kernel.org
> ---
>  include/uapi/linux/errqueue.h | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
> index 07bdce1f444a..b310b2c6d94f 100644
> --- a/include/uapi/linux/errqueue.h
> +++ b/include/uapi/linux/errqueue.h
> @@ -3,6 +3,12 @@
>
>  #include 
>
> +#ifdef __KERNEL__
> +#include 
> +#else
> +#include 
> +#endif /* __KERNEL__ */

This will break applications that include  manually.

I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where  is included after
.

  https://lkml.org/lkml/2016/9/12/872

The inverse will require changes to the libc header to avoid redefining
symbols already defined by 

The second patch in that 2-patch set included 
unconditionally after the fix. This broke builds that also included
 in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.

Re: [PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Willem de Bruijn]

On Sun, Aug 6, 2017 at 12:44 PM, Mikko Rapeli  wrote:
> 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 
> Cc: Willem de Bruijn 
> Cc: Soheil Hassas Yeganeh 
> Cc: net...@vger.kernel.org
> ---
>  include/uapi/linux/errqueue.h | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
> index 07bdce1f444a..b310b2c6d94f 100644
> --- a/include/uapi/linux/errqueue.h
> +++ b/include/uapi/linux/errqueue.h
> @@ -3,6 +3,12 @@
>
>  #include 
>
> +#ifdef __KERNEL__
> +#include 
> +#else
> +#include 
> +#endif /* __KERNEL__ */

This will break applications that include  manually.

I previously sent a patch to use libc-compat to make compilation succeed
when both are included in the case where  is included after
.

  https://lkml.org/lkml/2016/9/12/872

The inverse will require changes to the libc header to avoid redefining
symbols already defined by 

The second patch in that 2-patch set included 
unconditionally after the fix. This broke builds that also included
 in the wrong order. I did not resubmit the first patch as a
stand-alone, as it is not sufficient to avoid breakage.

[PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

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 
Cc: Willem de Bruijn 
Cc: Soheil Hassas Yeganeh 
Cc: net...@vger.kernel.org
---
 include/uapi/linux/errqueue.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
index 07bdce1f444a..b310b2c6d94f 100644
--- a/include/uapi/linux/errqueue.h
+++ b/include/uapi/linux/errqueue.h
@@ -3,6 +3,12 @@
 
 #include 
 
+#ifdef __KERNEL__
+#include 
+#else
+#include 
+#endif /* __KERNEL__ */
+
 struct sock_extended_err {
__u32   ee_errno;   
__u8ee_origin;
-- 
2.13.3

[PATCH v06 18/36] uapi linux/errqueue.h: include linux/time.h in user space

[Mikko Rapeli]

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 
Cc: Willem de Bruijn 
Cc: Soheil Hassas Yeganeh 
Cc: net...@vger.kernel.org
---
 include/uapi/linux/errqueue.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
index 07bdce1f444a..b310b2c6d94f 100644
--- a/include/uapi/linux/errqueue.h
+++ b/include/uapi/linux/errqueue.h
@@ -3,6 +3,12 @@
 
 #include 
 
+#ifdef __KERNEL__
+#include 
+#else
+#include 
+#endif /* __KERNEL__ */
+
 struct sock_extended_err {
__u32   ee_errno;   
__u8ee_origin;
-- 
2.13.3