Re: [LTP] [PATCH v2 4/4] syscalls: splice07: New splice tst_fd iterator test

[Jan Kara <jack@suse.cz>]

On Tue 24-10-23 09:56:47, Cyril Hrubis wrote:
> > > +	if (fd_in->type == TST_FD_PIPE_READ) {
> > > +		switch (fd_out->type) {
> > > +		case TST_FD_FILE:
> > > +		case TST_FD_PIPE_WRITE:
> > > +		case TST_FD_UNIX_SOCK:
> > > +		case TST_FD_INET_SOCK:
> > > +		case TST_FD_MEMFD:
> > > +			return;
> > > +		default:
> > > +		break;
> > > +		}
> > > +	}
> > > +
> > > +	if (fd_out->type == TST_FD_PIPE_WRITE) {
> > > +		switch (fd_in->type) {
> > > +		/* While these combinations succeeed */
> > > +		case TST_FD_FILE:
> > > +		case TST_FD_MEMFD:
> > > +			return;
> > > +		/* And this complains about socket not being connected */
> > > +		case TST_FD_INET_SOCK:
> > > +			return;
> > > +		default:
> > > +		break;
> > > +		}
> > > +	}
> > > +
> > > +	/* These produce EBADF instead of EINVAL */
> > > +	switch (fd_out->type) {
> > > +	case TST_FD_DIR:
> > > +	case TST_FD_DEV_ZERO:
> > > +	case TST_FD_PROC_MAPS:
> > > +	case TST_FD_INOTIFY:
> > > +	case TST_FD_PIPE_READ:
> > > +		exp_errno = EBADF;
> > > +	default:
> > > +	break;
> > > +	}
> > > +
> > > +	if (fd_in->type == TST_FD_PIPE_WRITE)
> > > +		exp_errno = EBADF;
> > > +
> > > +	if (fd_in->type == TST_FD_OPEN_TREE || fd_out->type == TST_FD_OPEN_TREE ||
> > > +	    fd_in->type == TST_FD_PATH || fd_out->type == TST_FD_PATH)
> > > +		exp_errno = EBADF;
> > 
> > This seems like something that could change due to checks changing
> > order.
> 
> I was hoping that kernel devs would look at the current state, which is
> documented in these conditions and tell me how shold we set the
> expectations. At least the open_tree() seems to differ from the rest in
> several cases, so maybe needs to be aligned with the rest.

Yeah, so the EINVAL vs EBADF vs EISDIR vs ESPIPE distinction is somewhat
arbitrary and as mentioned it very much depends on the order of checks we
do and that is not very consistent among different operations or over
longer time periods. So it would be good if tests could accept all errors
that make some sense. 

E.g. when we cannot seek (change file position) of the fd, ESPIPE is a
valid error return for any operation involving changing file position.
EISDIR is valid error for any directory fd when doing operation not expected
to work on directories. EINVAL and EBADF are quite generic and should be
accepted anytime fd is not suitable for the operation (generally we try to
return EBADF when the descriptor itself isn't suitable - e.g. O_PATH
descriptor, closed descriptor, ... - and return EINVAL when the open
*object* is not suitable but that is a very rough guideline people don't
always follow). EACCES / EPERM should be accepted error return when we
don't have enough permissions to perform operation on the fd. And so on.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR