lkml.org 
[lkml]   [2019]   [Mar]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 2/5] pid: add pidfd_open()
On Fri, Mar 29, 2019 at 4:54 PM Christian Brauner <christian@brauner.io> wrote:
> /* Introduction */
> This adds the pidfd_open() syscall.
> pidfd_open() allows to retrieve file descriptors for a given pid. This
> includes both file descriptors for processes and file descriptors for
> threads.

Looks good to me, overall. Apart from a few nits below:
Reviewed-by: Jann Horn <jannh@google.com>

[...]
> diff --git a/kernel/pid.c b/kernel/pid.c
> index 20881598bdfa..8c9e15e0e463 100644
> --- a/kernel/pid.c
> +++ b/kernel/pid.c
[...]
> +static struct file *pidfd_open_proc_pid(const struct file *procf, pid_t pid,
> + const struct pid *pidfd_pid)
> +{
> + char name[12]; /* int to strlen + \0 but with */

nit: comment suddenly ends at "but with"?

[...]
> +}
> +
> +static inline int pidfd_to_procfd(int procfd, struct file *pidfd_file)
> +{
> + long fd;

nit: This should probably be an int?

[...]
> + return fd;
> +}
[...]
> +static long pidfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + int procfd = arg;

nit: I think it'd be semantically cleaner to move this assignment into
the switch case, but I don't feel about it strongly.

> + switch (cmd) {
> + case PIDFD_GET_PROCFD:
> + return pidfd_to_procfd(procfd, file);
> + default:
> + return -ENOTTY;
> + }
> +}

\
 
 \ /
  Last update: 2019-03-30 00:47    [W:0.183 / U:0.896 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site