Re: [PATCH v2 2/5] pid: add pidfd_open()
From: Jann Horn <jannh@google.com>
Date: 2019-03-29 23:46:15
Also in:
lkml
On Fri, Mar 29, 2019 at 4:54 PM Christian Brauner [off-list ref] 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> [...]
quoted hunk ↗ jump to hunk
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;
+ }
+}