On Wed, Dec 4, 2024 at 12:35 PM Christian Brauner <brauner@xxxxxxxxxx> wrote: > > Add selftests for pidfs file handles. > > Link: https://lore.kernel.org/r/20241202-imstande-einsicht-d78753e1c632@brauner > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx> > --- > I've added a bunch more selftests. > Frankly, I'm going to probably be adding more as corner cases come to me. > And I'm just going to be amending the patch and stuffing them into the > tree unless I hear objections. > --- Generally, tests look good and you may add Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx> But I wonder... [...] > +/* > + * Test valid flags to open a pidfd file handle. Note, that > + * PIDFD_NONBLOCK is defined as O_NONBLOCK and O_NONBLOCK is an alias to > + * O_NDELAY. Also note that PIDFD_THREAD is an alias for O_EXCL. > + */ > +TEST_F(file_handle, open_by_handle_at_valid_flags) > +{ > + int mnt_id; > + struct file_handle *fh; > + int pidfd = -EBADF; > + struct stat st1, st2; > + > + fh = malloc(sizeof(struct file_handle) + MAX_HANDLE_SZ); > + ASSERT_NE(fh, NULL); > + memset(fh, 0, sizeof(struct file_handle) + MAX_HANDLE_SZ); > + fh->handle_bytes = MAX_HANDLE_SZ; > + > + ASSERT_EQ(name_to_handle_at(self->child_pidfd2, "", fh, &mnt_id, AT_EMPTY_PATH), 0); > + > + ASSERT_EQ(fstat(self->child_pidfd2, &st1), 0); > + > + pidfd = open_by_handle_at(self->pidfd, fh, > + O_RDONLY | > + O_WRONLY | > + O_RDWR | > + O_NONBLOCK | > + O_NDELAY | > + O_CLOEXEC | > + O_EXCL); > + ASSERT_GE(pidfd, 0); > + IIRC, your patch always opens an fd with O_RDWR. Right? Isn't it confusing to request O_RDONLY and get O_RDWR? Should we only allow requests to request open_by_handle_at() with O_RDWR mode? Thanks, Amir.
