Re: [PATCH 2/3] syscalls/readahead01: Make use of tst_fd_iterate()

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > TODO: readahead() on /proc/self/maps seems to succeed is that to be
> >       expected?
> 
> Not sure.
> How does llseek() work on the same fd?

Looks like we we can seek in that file as well, accordingly to man pages
we cannot seek in pipe, socket, and fifo, which seems to match the
reality.  We can apparently seek in O_DIRECTORY fd as well, not sure if
that is even useful.

> > -static void test_invalid_fd(void)
> >  {
> >         int fd[2];
> >
> > -       tst_res(TINFO, "%s pipe", __func__);
> > +       TST_EXP_FAIL(readahead(-1, 0, getpagesize()), EBADF,
> > +                    "readahead() with fd = -1");
> > +
> 
> Any reason not to include a bad and a closed fd in the iterator?

I wanted to avoid mixing valid and invalid fds because we tend to get
different errnos for these, since the situation is different between
"this is not a file descriptor" and "this is not supported on this kind
of file descriptor".

> >         SAFE_PIPE(fd);
> > -       TST_EXP_FAIL(readahead(fd[0], 0, getpagesize()), EINVAL);
> >         SAFE_CLOSE(fd[0]);
> >         SAFE_CLOSE(fd[1]);
> >
> > -       tst_res(TINFO, "%s socket", __func__);
> > -       fd[0] = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
> > -       TST_EXP_FAIL(readahead(fd[0], 0, getpagesize()), EINVAL);
> > -       SAFE_CLOSE(fd[0]);
> > +       TST_EXP_FAIL(readahead(fd[0], 0, getpagesize()), EBADF,
> > +                    "readahead() with invalid fd");
> > +}
> > +
> > +static void test_invalid_fd(struct tst_fd *fd)
> > +{
> > +       switch (fd->type) {
> > +       case TST_FD_FILE:
> > +       case TST_FD_PIPE_OUT:
> > +               return;
> > +       default:
> > +               break;
> > +       }
> > +
> > +       TST_EXP_FAIL(readahead(fd->fd, 0, getpagesize()), EINVAL,
> > +                    "readahead() on %s", tst_fd_desc(fd));
> 
> Thinking forward and we would like to change this error code to ESPIPE
> is there already a helper to expect one of a few error codes?

Not yet. The hardest part is again figuring out right API. We usually
try to check for the new behavior on newer kernels, which would be
complex to encode into the parameters, so maybe we just need to pass a
callback that would return the right errno. Maybe something as:

static int exp_errno(void)
{
	if (tst_kvercmp(6, 7, 0) >= 0)
		return ESPIPE;

	return EINVAL;
}

...
	TST_EXP_FAIL_CB(readahead(fd->fd, 0, getpagesize()), exp_errno,
		"readahead() on %s", tst_fd_desc(fd));
...

-- 
Cyril Hrubis
chrubis@suse.cz