diff mbox series

xfs: remove incorrect ASSERT in xfs_rename

Message ID bbb4b6d5-744c-11c8-fcda-62777e8d7b19@redhat.com (mailing list archive)
State Accepted, archived
Headers show
Series xfs: remove incorrect ASSERT in xfs_rename | expand

Commit Message

Eric Sandeen Dec. 1, 2021, 5:17 a.m. UTC 
This ASSERT in xfs_rename is a) incorrect, because
(RENAME_WHITEOUT|RENAME_NOREPLACE) is a valid combination, and
b) unnecessary, because actual invalid flag combinations are already
handled at the vfs level in do_renameat2() before we get called.
So, remove it.

Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

Comments

Eric Sandeen Dec. 1, 2021, 4:42 p.m. UTC | #1 
On 11/30/21 11:17 PM, Eric Sandeen wrote:
> This ASSERT in xfs_rename is a) incorrect, because
> (RENAME_WHITEOUT|RENAME_NOREPLACE) is a valid combination, and
> b) unnecessary, because actual invalid flag combinations are already
> handled at the vfs level in do_renameat2() before we get called.
> So, remove it.
> 
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 64b9bf3..6771f35 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3122,7 +3122,6 @@ struct xfs_iunlink {
>        * appropriately.
>        */
>       if (flags & RENAME_WHITEOUT) {
> -        ASSERT(!(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)));
>           error = xfs_rename_alloc_whiteout(mnt_userns, target_dp, &wip);
>           if (error)
>               return error;
> 

Ugh, I don't understand this flavor of whitespace mangling I'm getting now,
I'm sorry :(
Darrick J. Wong Dec. 1, 2021, 5:57 p.m. UTC | #2 
On Tue, Nov 30, 2021 at 11:17:56PM -0600, Eric Sandeen wrote:
> This ASSERT in xfs_rename is a) incorrect, because
> (RENAME_WHITEOUT|RENAME_NOREPLACE) is a valid combination, and
> b) unnecessary, because actual invalid flag combinations are already
> handled at the vfs level in do_renameat2() before we get called.
> So, remove it.
> 
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

LGTM,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 64b9bf3..6771f35 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -3122,7 +3122,6 @@ struct xfs_iunlink {
>  	 * appropriately.
>  	 */
>  	if (flags & RENAME_WHITEOUT) {
> -		ASSERT(!(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)));
>  		error = xfs_rename_alloc_whiteout(mnt_userns, target_dp, &wip);
>  		if (error)
>  			return error;
>
diff mbox series

Patch

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 64b9bf3..6771f35 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -3122,7 +3122,6 @@  struct xfs_iunlink {
  	 * appropriately.
  	 */
  	if (flags & RENAME_WHITEOUT) {
-		ASSERT(!(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)));
  		error = xfs_rename_alloc_whiteout(mnt_userns, target_dp, &wip);
  		if (error)
  			return error;