Re: + uapi-fix-linux-sysctlh-userspace-compilation-errors.patch added to -mm tree

Re: + uapi-fix-linux-sysctlh-userspace-compilation-errors.patch added to -mm tree

[Alexey Dobriyan]

On Thu, Feb 23, 2017 at 02:44:31PM -0800, akpm@linux-foundation.org wrote:
> /usr/include/linux/sysctl.h:38:2: error: unknown type name 'size_t'
>   size_t *oldlenp;
> /usr/include/linux/sysctl.h:40:2: error: unknown type name 'size_t'
>   size_t newlen;

> --- a/include/uapi/linux/sysctl.h~uapi-fix-linux-sysctlh-userspace-compilation-errors
> +++ a/include/uapi/linux/sysctl.h
> @@ -26,6 +26,10 @@
>  #include <linux/types.h>
>  #include <linux/compiler.h>
>  
> +#ifndef __KERNEL__
> +#include <stddef.h>		/* For size_t. */
> +#endif

There is __kernel_size_t for cases like this.

Re: + uapi-fix-linux-sysctlh-userspace-compilation-errors.patch added to -mm tree

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1158 bytes --]

On Fri, Feb 24, 2017 at 05:35:33PM +0300, Alexey Dobriyan wrote:
> On Thu, Feb 23, 2017 at 02:44:31PM -0800, akpm@linux-foundation.org wrote:
> > /usr/include/linux/sysctl.h:38:2: error: unknown type name 'size_t'
> >   size_t *oldlenp;
> > /usr/include/linux/sysctl.h:40:2: error: unknown type name 'size_t'
> >   size_t newlen;
> 
> > --- a/include/uapi/linux/sysctl.h~uapi-fix-linux-sysctlh-userspace-compilation-errors
> > +++ a/include/uapi/linux/sysctl.h
> > @@ -26,6 +26,10 @@
> >  #include <linux/types.h>
> >  #include <linux/compiler.h>
> >  
> > +#ifndef __KERNEL__
> > +#include <stddef.h>		/* For size_t. */
> > +#endif
> 
> There is __kernel_size_t for cases like this.

No, __kernel_size_t is not for cases like this because size_t differs
from __kernel_size_t on x32 and mips n32.

Fortunately, there is no sysctl syscall entry on x32, but on mips m32
it is there with number 6152, implemented using compat_sys_sysctl.
The latter operates with argument of type struct compat_sysctl_args*
where newlen is of type compat_size_t.

If you change it from size_t to __kernel_size_t, you'll break mips n32.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 801 bytes --]

Re: + uapi-fix-linux-sysctlh-userspace-compilation-errors.patch added to -mm tree

[Alexey Dobriyan]

On Sat, Feb 25, 2017 at 1:35 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Fri, Feb 24, 2017 at 05:35:33PM +0300, Alexey Dobriyan wrote:
>> On Thu, Feb 23, 2017 at 02:44:31PM -0800, akpm@linux-foundation.org wrote:
>> > /usr/include/linux/sysctl.h:38:2: error: unknown type name 'size_t'
>> >   size_t *oldlenp;
>> > /usr/include/linux/sysctl.h:40:2: error: unknown type name 'size_t'
>> >   size_t newlen;
>>
>> > --- a/include/uapi/linux/sysctl.h~uapi-fix-linux-sysctlh-userspace-compilation-errors
>> > +++ a/include/uapi/linux/sysctl.h
>> > @@ -26,6 +26,10 @@
>> >  #include <linux/types.h>
>> >  #include <linux/compiler.h>
>> >
>> > +#ifndef __KERNEL__
>> > +#include <stddef.h>                /* For size_t. */
>> > +#endif
>>
>> There is __kernel_size_t for cases like this.
>
> No, __kernel_size_t is not for cases like this because size_t differs
> from __kernel_size_t on x32 and mips n32.
>
> Fortunately, there is no sysctl syscall entry on x32, but on mips m32
> it is there with number 6152, implemented using compat_sys_sysctl.
> The latter operates with argument of type struct compat_sysctl_args*
> where newlen is of type compat_size_t.
>
> If you change it from size_t to __kernel_size_t, you'll break mips n32.

You're gluing kernel interface to an userspace header
which kernel doesn't control. How can this fix anything?

Re: + uapi-fix-linux-sysctlh-userspace-compilation-errors.patch added to -mm tree

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 2170 bytes --]

On Mon, Feb 27, 2017 at 12:38:16PM +0300, Alexey Dobriyan wrote:
> On Sat, Feb 25, 2017 at 1:35 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> > On Fri, Feb 24, 2017 at 05:35:33PM +0300, Alexey Dobriyan wrote:
> >> On Thu, Feb 23, 2017 at 02:44:31PM -0800, akpm@linux-foundation.org wrote:
> >> > /usr/include/linux/sysctl.h:38:2: error: unknown type name 'size_t'
> >> >   size_t *oldlenp;
> >> > /usr/include/linux/sysctl.h:40:2: error: unknown type name 'size_t'
> >> >   size_t newlen;
> >>
> >> > --- a/include/uapi/linux/sysctl.h~uapi-fix-linux-sysctlh-userspace-compilation-errors
> >> > +++ a/include/uapi/linux/sysctl.h
> >> > @@ -26,6 +26,10 @@
> >> >  #include <linux/types.h>
> >> >  #include <linux/compiler.h>
> >> >
> >> > +#ifndef __KERNEL__
> >> > +#include <stddef.h>                /* For size_t. */
> >> > +#endif
> >>
> >> There is __kernel_size_t for cases like this.
> >
> > No, __kernel_size_t is not for cases like this because size_t differs
> > from __kernel_size_t on x32 and mips n32.
> >
> > Fortunately, there is no sysctl syscall entry on x32, but on mips m32
> > it is there with number 6152, implemented using compat_sys_sysctl.
> > The latter operates with argument of type struct compat_sysctl_args*
> > where newlen is of type compat_size_t.
> >
> > If you change it from size_t to __kernel_size_t, you'll break mips n32.
> 
> You're gluing kernel interface to an userspace header
> which kernel doesn't control.

Are you sure it's me who does the gluing?

As no uapi header provides a definition of size_t, they are essentially
relying on size_t being provided elsewhere.

For this reason, inclusion of <stddef.h> is the most conservative fix.

> How can this fix anything?

For example,
AC_CHECK_HEADERS([<linux/sysctl.h>]) doesn't work, one has to use
AC_CHECK_HEADERS([<linux/sysctl.h>], , , [#include <stddef.h>]) instead.
The same issue arises with other autoconf checks.

I think that every uapi header file should be standalone, i.e. it must
include, directly or indirectly, every definition it uses, so that no user
should have to guess header dependencies.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 801 bytes --]

Re: + uapi-fix-linux-sysctlh-userspace-compilation-errors.patch added to -mm tree

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

On Sat, Feb 25, 2017 at 01:35:51AM +0300, Dmitry V. Levin wrote:
> On Fri, Feb 24, 2017 at 05:35:33PM +0300, Alexey Dobriyan wrote:
> > On Thu, Feb 23, 2017 at 02:44:31PM -0800, akpm@linux-foundation.org wrote:
> > > /usr/include/linux/sysctl.h:38:2: error: unknown type name 'size_t'
> > >   size_t *oldlenp;
> > > /usr/include/linux/sysctl.h:40:2: error: unknown type name 'size_t'
> > >   size_t newlen;
> > 
> > > --- a/include/uapi/linux/sysctl.h~uapi-fix-linux-sysctlh-userspace-compilation-errors
> > > +++ a/include/uapi/linux/sysctl.h
> > > @@ -26,6 +26,10 @@
> > >  #include <linux/types.h>
> > >  #include <linux/compiler.h>
> > >  
> > > +#ifndef __KERNEL__
> > > +#include <stddef.h>		/* For size_t. */
> > > +#endif
> > 
> > There is __kernel_size_t for cases like this.
> 
> No, __kernel_size_t is not for cases like this because size_t differs
> from __kernel_size_t on x32 and mips n32.
> 
> Fortunately, there is no sysctl syscall entry on x32, but on mips m32
> it is there with number 6152, implemented using compat_sys_sysctl.
> The latter operates with argument of type struct compat_sysctl_args*
> where newlen is of type compat_size_t.
> 
> If you change it from size_t to __kernel_size_t, you'll break mips n32.

Apparently, my statement that sizeof(size_t) < sizeof(__kernel_size_t)
on mips n32 is not correct, so x32 is the only architecture of this kind.

This means that size_t can be replaced with __kernel_size_t in this
particular case.  Does anybody want that change instead of my original
proposal?


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 801 bytes --]