Re: [LTP] [PATCH 2/2] syscalls/fanotify21: add new test checking the returned pidfd from fanotify in FAN_REPORT_PIDFD mode

[Matthew Bobrowski via ltp <ltp@lists.linux.it>]

On Wed, Oct 27, 2021 at 01:35:53PM +0300, Amir Goldstein wrote:
> On Wed, Oct 27, 2021 at 12:45 PM Matthew Bobrowski <repnop@google.com> wrote:
> >
> > A new test that performs verification on the values returned within the
> > struct fanotify_event_info_pidfd record when notification group intialized
> > in FAN_REPORT_PIDFD mode.
> >
> > Signed-off-by: Matthew Bobrowski <repnop@google.com>
> > ---
> >  testcases/kernel/syscalls/fanotify/.gitignore |   1 +
> >  .../kernel/syscalls/fanotify/fanotify21.c     | 363 ++++++++++++++++++
> >  2 files changed, 364 insertions(+)
> >  create mode 100644 testcases/kernel/syscalls/fanotify/fanotify21.c
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
> > index c99e6fff7..35e73b91e 100644
> > --- a/testcases/kernel/syscalls/fanotify/.gitignore
> > +++ b/testcases/kernel/syscalls/fanotify/.gitignore
> > @@ -18,4 +18,5 @@
> >  /fanotify18
> >  /fanotify19
> >  /fanotify20
> > +/fanotify21
> >  /fanotify_child
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
> > new file mode 100644
> > index 000000000..f64f8fef4
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
> > @@ -0,0 +1,363 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2021 Google. All Rights Reserved.
> > + *
> > + * Started by Matthew Bobrowski <repnop@google.com>
> > + */
> > +
> > +/*\
> > + * [Description]
> > + *
> > + * A test which verifies whether the returned struct
> > + * fanotify_event_info_pidfd in FAN_REPORT_PIDFD mode contains the
> > + * expected set of information.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <stdio.h>
> > +#include <ctype.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include "tst_test.h"
> > +#include "tst_safe_stdio.h"
> > +#include "lapi/pidfd_open.h"
> > +
> > +#ifdef HAVE_SYS_FANOTIFY_H
> > +#include "fanotify.h"
> > +
> > +#define BUF_SZ         4096
> > +#define MOUNT_PATH     "fs_mnt"
> > +#define TEST_FILE      MOUNT_PATH "/testfile"
> > +
> > +struct pidfd_fdinfo_t {
> > +       int pos;
> > +       int flags;
> > +       int mnt_id;
> > +       int pid;
> > +       int ns_pid;
> > +};
> > +
> > +struct test_case_t {
> > +       char *name;
> > +       int fork;
> > +       int want_pidfd_err;
> > +} test_cases[] = {
> > +       {
> > +               "return a valid pidfd for event created by self",
> > +               0,
> > +               0,
> > +       },
> > +       {
> > +               "return invalid pidfd for event created by terminated child",
> > +               1,
> > +               FAN_NOPIDFD,
> > +       },
> > +};
> > +
> > +static int fanotify_fd;
> > +static char event_buf[BUF_SZ];
> > +static struct pidfd_fdinfo_t *self_pidfd_fdinfo = NULL;
> > +
> > +static char *trim(char *line)
> > +{
> > +       char *start = line;
> > +       char *end = line + strlen(line);
> > +
> > +       while(*start && isspace(*start))
> > +               start++;
> > +
> > +       while(end > start && isspace(*(end - 1)))
> > +               end--;
> > +
> > +       *end = '\0';
> > +       return start;
> > +}
> > +
> > +static int parse_pidfd_fdinfo_line(char *line,
> > +                                  struct pidfd_fdinfo_t *pidfd_fdinfo)
> > +{
> > +       char *ptr, *key, *value;
> > +
> > +       ptr = strchr(line, ':');
> > +       if (ptr == NULL)
> > +               return -1;
> > +
> > +       *ptr++ = '\0';
> > +       key = trim(line);
> > +       value = trim(ptr);
> > +
> > +       /*
> > +        * Ensure to cover all keys of interest that may be found within the
> > +        * pidfd fdinfo. If we encounter an unexpected key, skip it.
> > +        */
> > +       if (strcmp(key, "pos") == 0)
> > +               pidfd_fdinfo->pos = atoi(value);
> > +       else if (strcmp(key, "flags") == 0)
> > +               pidfd_fdinfo->flags = (int)strtol(value, NULL, 16);
> > +       else if (strcmp(key, "mnt_id") == 0)
> > +               pidfd_fdinfo->mnt_id = atoi(value);
> > +       else if (strcmp(key, "Pid") == 0)
> > +               pidfd_fdinfo->pid = atoi(value);
> > +       else if (strcmp(key, "NSpid") == 0)
> > +               pidfd_fdinfo->ns_pid = atoi(value);
> > +
> > +       return 0;
> > +}
> 
> Please use existing LTP parsing utilities.
> It's never a good idea to re-implement these sort of things.
> With a quick grep I found:
> 
> SAFE_FILE_LINES_SCANF("/proc/meminfo", "SwapCached: %ld",
> 
> Otherwise, test looks fine to me.

Ah, wonderful. I wasn't aware that this helper had existed. A failure
on my part for not properly checking. I think this will work the
treat!

/M

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp