[PATCH v2 02/11] linux: Extend internal clone3 documentation

[Carlos O'Donell]

On 11/4/22 15:01, Adhemerval Zanella via Libc-alpha wrote:
> Different than kernel, clone3 returns EINVAL for NULL struct
> clone_args or function pointer.  This is similar to clone
> interface that return EINVAL for NULL function argument.

Please post v4.

Suggest:

Document that the internal __clone3 returns EINVAL for a NULL struct
clone_args or function pointer and that this differs from the kernel. 
This behavior is similar to the clone interface that returns EINVAL
for a NULL function argument.
 
> It also clean up the Linux clone3.h interface, since it not
> currently exported.

Suggest:

Additionally cleanup the Linux clone3.h interface (not currently exported).

> 
> Checked on x86_64-linux-gnu.
> ---
>  include/clone_internal.h         | 24 +++++++++++++++++++-----
>  sysdeps/unix/sysv/linux/clone3.h | 10 +---------
>  2 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/include/clone_internal.h b/include/clone_internal.h
> index 4b23ef33ce..73b8114df4 100644
> --- a/include/clone_internal.h
> +++ b/include/clone_internal.h
> @@ -1,10 +1,24 @@
> -#ifndef _CLONE3_H
> -#include_next <clone3.h>
> +#ifndef _CLONE_INTERNAL_H
> +#define _CLONE_INTERNAL_H
>  
> -extern __typeof (clone3) __clone3;
> +#include <clone3.h>
>  
> -/* The internal wrapper of clone/clone2 and clone3.  If __clone3 returns
> -   -1 with ENOSYS, fall back to clone or clone2.  */
> +/* The clone3 syscall provides a superset of the functionality of the clone
> +   interface.  The kernel might extend __CL_ARGS struct in the future, with
> +   each version with a diffent __SIZE.  If the child is created, it will
> +   start __FUNC function with __ARG arguments.

OK.

> +
> +   Different than kernel, the implementation also returns EINVAL for an
> +   invalid NULL __CL_ARGS or __FUNC (similar to __clone).

Suggest:

Unlike the kernel the implementation returns EINVAL for an invalid NULL __CL_ARGS
or __FUNC (similar to __clone).

> +

This has nothing to do with the ABI in glibc so I'd avoid "ABI" here.

Suggest:

This function is only implemented if the machine defines HAVE_CLONE3_WRAPPER.

> +   This function is only implemented if the ABI defines HAVE_CLONE3_WRAPPER.



> +*/
> +extern int __clone3 (struct clone_args *__cl_args, size_t __size,
> +		     int (*__func) (void *__arg), void *__arg);
> +
> +/* The internal wrapper of clone/clone2 and clone3.  Different than __clone3,
> +   it will align the stack if required.  If __clone3 returns -1 with ENOSYS,
> +   fall back to clone or clone2.  */
>  extern int __clone_internal (struct clone_args *__cl_args,
>  			     int (*__func) (void *__arg), void *__arg);
>  
> diff --git a/sysdeps/unix/sysv/linux/clone3.h b/sysdeps/unix/sysv/linux/clone3.h
> index 889014a6a9..7c7e2cdcb0 100644
> --- a/sysdeps/unix/sysv/linux/clone3.h
> +++ b/sysdeps/unix/sysv/linux/clone3.h
> @@ -1,4 +1,4 @@
> -/* The wrapper of clone3.
> +/* The clone3 kernel interface definitions.
>     Copyright (C) 2021-2022 Free Software Foundation, Inc.
>     This file is part of the GNU C Library.
>  
> @@ -23,8 +23,6 @@
>  #include <stddef.h>
>  #include <bits/types.h>
>  
> -__BEGIN_DECLS
> -
>  /* The unsigned 64-bit and 8-byte aligned integer type.  */
>  typedef __U64_TYPE __aligned_uint64_t __attribute__ ((__aligned__ (8)));
>  
> @@ -58,10 +56,4 @@ struct clone_args
>    __aligned_uint64_t cgroup;
>  };
>  
> -/* The wrapper of clone3.  */
> -extern int clone3 (struct clone_args *__cl_args, size_t __size,
> -		   int (*__func) (void *__arg), void *__arg);
> -
> -__END_DECLS
> -

OK.

>  #endif /* clone3.h */

-- 
Cheers,
Carlos.