diff mbox series

[1/2] linux: fix/improve clone_args description

Message ID 20240716224136.784105-1-kolyshkin@gmail.com (mailing list archive)
State Changes Requested
Headers
Series [1/2] linux: fix/improve clone_args description |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Kir Kolyshkin July 16, 2024, 10:41 p.m. UTC 
  Fixes:
 - pidfd type is *int (as it's a file descriptor), not pid_t;
 - parent_tid type is *pid_t (as it's a TID), not int.

While at it, describe which flags are needed to be set to use some of
these fields.

Fixes: d8ea0d0168 ("Add an internal wrapper for clone, clone2 and clone3")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 sysdeps/unix/sysv/linux/clone3.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Adhemerval Zanella Netto July 19, 2024, 1:28 p.m. UTC | #1 
On 16/07/24 19:41, Kir Kolyshkin wrote:
> Fixes:
>  - pidfd type is *int (as it's a file descriptor), not pid_t;
>  - parent_tid type is *pid_t (as it's a TID), not int.
> 
> While at it, describe which flags are needed to be set to use some of
> these fields.
> 
> Fixes: d8ea0d0168 ("Add an internal wrapper for clone, clone2 and clone3")
> Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Some minor style issue below.

> ---
>  sysdeps/unix/sysv/linux/clone3.h | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/clone3.h b/sysdeps/unix/sysv/linux/clone3.h
> index deff63da2a..d097ed7502 100644
> --- a/sysdeps/unix/sysv/linux/clone3.h
> +++ b/sysdeps/unix/sysv/linux/clone3.h
> @@ -41,25 +41,28 @@ struct clone_args
>  {
>    /* Flags bit mask.  */
>    __aligned_uint64_t flags;
> -  /* Where to store PID file descriptor (pid_t *).  */
> +  /* Where to store PID file descriptor (int *). Needs CLONE_PIDFD flag set. */

Double space period.

>    __aligned_uint64_t pidfd;
> -  /* Where to store child TID, in child's memory (pid_t *).  */
> +  /* Where to store child TID, in child's memory (pid_t *).
> +   * Needs CLONE_CHILD_SETTID flag set. */

Multi line comments should not start with '*':

  /* Start ...
     ... end.  */

>    __aligned_uint64_t child_tid;
> -  /* Where to store child TID, in parent's memory (int *). */
> +  /* Where to store child TID, in parent's memory (pid_t *).
> +   * Needs CLONE_PARENT_SETTID flag set. */
>    __aligned_uint64_t parent_tid;
> -  /* Signal to deliver to parent on child termination */
> +  /* Signal to deliver to parent on child termination. */

Double space after period.

>    __aligned_uint64_t exit_signal;
>    /* The lowest address of stack.  */
>    __aligned_uint64_t stack;
>    /* Size of stack.  */
>    __aligned_uint64_t stack_size;
> -  /* Location of new TLS.  */
> +  /* Location of new TLS. Needs CLONE_SETTLS flag set. */
>    __aligned_uint64_t tls;
>    /* Pointer to a pid_t array (since Linux 5.5).  */
>    __aligned_uint64_t set_tid;
>    /* Number of elements in set_tid (since Linux 5.5). */
>    __aligned_uint64_t set_tid_size;
> -  /* File descriptor for target cgroup of child (since Linux 5.7).  */
> +  /* File descriptor for target cgroup of child (since Linux 5.7).
> +   * Needs CLONE_INTO_CGROUP flag set. */
>    __aligned_uint64_t cgroup;
>  };
>
diff mbox series

Patch

diff --git a/sysdeps/unix/sysv/linux/clone3.h b/sysdeps/unix/sysv/linux/clone3.h
index deff63da2a..d097ed7502 100644
--- a/sysdeps/unix/sysv/linux/clone3.h
+++ b/sysdeps/unix/sysv/linux/clone3.h
@@ -41,25 +41,28 @@  struct clone_args
 {
   /* Flags bit mask.  */
   __aligned_uint64_t flags;
-  /* Where to store PID file descriptor (pid_t *).  */
+  /* Where to store PID file descriptor (int *). Needs CLONE_PIDFD flag set. */
   __aligned_uint64_t pidfd;
-  /* Where to store child TID, in child's memory (pid_t *).  */
+  /* Where to store child TID, in child's memory (pid_t *).
+   * Needs CLONE_CHILD_SETTID flag set. */
   __aligned_uint64_t child_tid;
-  /* Where to store child TID, in parent's memory (int *). */
+  /* Where to store child TID, in parent's memory (pid_t *).
+   * Needs CLONE_PARENT_SETTID flag set. */
   __aligned_uint64_t parent_tid;
-  /* Signal to deliver to parent on child termination */
+  /* Signal to deliver to parent on child termination. */
   __aligned_uint64_t exit_signal;
   /* The lowest address of stack.  */
   __aligned_uint64_t stack;
   /* Size of stack.  */
   __aligned_uint64_t stack_size;
-  /* Location of new TLS.  */
+  /* Location of new TLS. Needs CLONE_SETTLS flag set. */
   __aligned_uint64_t tls;
   /* Pointer to a pid_t array (since Linux 5.5).  */
   __aligned_uint64_t set_tid;
   /* Number of elements in set_tid (since Linux 5.5). */
   __aligned_uint64_t set_tid_size;
-  /* File descriptor for target cgroup of child (since Linux 5.7).  */
+  /* File descriptor for target cgroup of child (since Linux 5.7).
+   * Needs CLONE_INTO_CGROUP flag set. */
   __aligned_uint64_t cgroup;
 };