On Fri, Jan 17, 2014 at 11:53:07PM +1300, Michael Kerrisk (man-pages) wrote: > Hi Miklos, > > A few comments below, including one piece in the code that really must be > fixed. > > On 01/16/2014 11:54 PM, Miklos Szeredi wrote: > >> On Wed, Jan 15, 2014 at 7:23 PM, J. Bruce Fields <bfie...@fieldses.org> > >> wrote: > >>> Do you have a man page update somewhere for the two new flags? > > > > Here's the updated man page (and attached the patch) > > > > Michael, could you please review the interface? > > > > I forgot to CC you when posing the patch series. I can resend it if you > > want, > > or you can fetch the latest version of the cross-rename series from: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/vfs.git > > cross-rename > > [...] > > > renameat2() has an additional flags argument. renameat2() call > > with a > > zero flags argument is equivalent to renameat(). > > > > The flags argument is a bitfield consisting of zero or more of the > > fol- > > lowing constants defined in <linux/fs.h>: > > > > RENAME_NOREPLACE > > Don't overwrite the target of the rename. Return an error > > if > > the target would be overwritten. > > > > RENAME_EXCHANGE > > Atomically exchange the source and destination. Both must > > exist > > but may be of a different type (e.g. one a non-empty > > directory > > and the other a symbolic link). > > Somewhere here it would be good to explain the consequences if > > (flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)) == > (RENAME_NOREPLACE | RENAME_EXCHANGE) > > Okay -- it's EINVAL, but here the man page text should say something like > "these two flags can't be specified together", right? > > > RETURN VALUE > > On success, renameat() and renameat2() return 0. On error, -1 > > is > > returned and errno is set to indicate the error. > > > > ERRORS > > The same errors that occur for rename(2) can also occur for > > renameat() > > and renameat2(). The following additional errors can occur > > for > > renameat() and renameat2(): > > > > EBADF olddirfd or newdirfd is not a valid file descriptor. > > > > ENOTDIR > > oldpath is relative and olddirfd is a file descriptor > > referring > > to a file other than a directory; or similar for newpath > > and > > newdirfd > > > > The following additional errors are defined for renameat2(): > > > > EOPNOTSUPP > > The filesystem does not support a flag in flags > > This is not the usual error for an invalid bit flag. Please make it EINVAL.
I agree that EINVAL makes sense for an invalid bit flag. But renameat2() can also fail when the caller passes a perfectly valid flags field but the paths resolve to a filesystem that doesn't support the RENAME_EXCHANGE operation. EOPNOTSUPP looks more appropriate in that case. > (See the man pages for the *at() calls that have a 'flags" argument.) Aren't those flags mostly handled in the vfs? In which case they work everywhere, so there isn't the same distinction between "flag is defined" and "behavior requested by flag is unsupported for the given objects". --b. > > EINVAL Invalid combination of flags > > (This is okay.) > > Looks otherwise okay to me (and I agree with Bruce's comments). > > Cheers, > > Michael > > > > -- > Michael Kerrisk > Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/ > Linux/UNIX System Programming Training: http://man7.org/training/ > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
