Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752199AbaAQKxS (ORCPT ); Fri, 17 Jan 2014 05:53:18 -0500 Received: from mail-bk0-f46.google.com ([209.85.214.46]:59306 "EHLO mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751540AbaAQKxO (ORCPT ); Fri, 17 Jan 2014 05:53:14 -0500 Message-ID: <52D90B93.5010209@gmail.com> Date: Fri, 17 Jan 2014 23:53:07 +1300 From: "Michael Kerrisk (man-pages)" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: Miklos Szeredi CC: mtk.manpages@gmail.com, "J. Bruce Fields" , Jan Kara , Al Viro , Linus Torvalds , Linux-Fsdevel , Kernel Mailing List , Christoph Hellwig , Andrew Morton , David Howells , Zach Brown , Andy Lutomirski , "mszeredi@suse.cz" Subject: Re: [PATCH 11/11] ext4: add cross rename support References: <1389219015-10980-1-git-send-email-miklos@szeredi.hu> <1389219015-10980-12-git-send-email-miklos@szeredi.hu> <20140113122517.GA10366@quack.suse.cz> <20140115182347.GB5715@fieldses.org> <20140116105406.GF24171@tucsk.piliscsaba.szeredi.hu> In-Reply-To: <20140116105406.GF24171@tucsk.piliscsaba.szeredi.hu> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 : > > 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. (See the man pages for the *at() calls that have a 'flags" argument.) > 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/