Re: [PATCH 02/25] iov_iter: Use accessor function [ver #2]

From: NeilBrown <neilb@suse.com>
To: David Howells <dhowells@redhat.com>, viro@zeniv.linux.org.uk
Cc: dhowells@redhat.com, linux-afs@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/25] iov_iter: Use accessor function [ver #2]
Date: Wed, 28 Nov 2018 12:39:38 +1100	[thread overview]
Message-ID: <87efb6ou91.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <154033907731.12041.15677154092495801379.stgit@warthog.procyon.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2675 bytes --]

On Wed, Oct 24 2018, David Howells wrote:

> Use accessor functions to access an iterator's type and direction.  This
> allows for the possibility of using some other method of determining the
> type of iterator than if-chains with bitwise-AND conditions.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
....
> @@ -74,7 +104,8 @@ static inline struct iovec iov_iter_iovec(const struct iov_iter *iter)
>  }
>  
>  #define iov_for_each(iov, iter, start)				\
> -	if (!((start).type & (ITER_BVEC | ITER_PIPE)))		\
> +	if (iov_iter_type(start) == ITER_IOVEC ||		\
> +	    iov_iter_type(start) == ITER_KVEC)			\
>  	for (iter = (start);					\
>  	     (iter).count &&					\
>  	     ((iov = iov_iter_iovec(&(iter))), 1);		\

BTW this breaks iov_for_each().
'start' is a struct, but iov_iter_type() requires a pointer to a struct.
You could fix it with
> +	if (iov_iter_type(&start) == ITER_IOVEC ||		\
> +	    iov_iter_type(&start) == ITER_KVEC)			\

but as there are no users it is probably best to discard it.
I discovered this because lustre uses (or rather "used") it.

NeilBrown

From: NeilBrown <neilb@suse.com>
Date: Wed, 28 Nov 2018 12:38:30 +1100
Subject: [PATCH] iov_iter: discard iov_for_each()

iov_for_each has no users and cannot compile
as 'start' is treated sometimes like a struct
and sometimes like a pointer to a struct.

So discard it.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 .clang-format       | 1 -
 include/linux/uio.h | 8 --------
 2 files changed, 9 deletions(-)

diff --git a/.clang-format b/.clang-format
index e6080f5834a3..c144d9c24d5d 100644
--- a/.clang-format
+++ b/.clang-format
@@ -259,7 +259,6 @@ ForEachMacros:
   - 'idr_for_each_entry_ul'
   - 'inet_bind_bucket_for_each'
   - 'inet_lhash2_for_each_icsk_rcu'
-  - 'iov_for_each'
   - 'key_for_each'
   - 'key_for_each_safe'
   - 'klp_for_each_func'
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 55ce99ddb912..a2b2109838b0 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -109,14 +109,6 @@ static inline struct iovec iov_iter_iovec(const struct iov_iter *iter)
 	};
 }
 
-#define iov_for_each(iov, iter, start)				\
-	if (iov_iter_type(start) == ITER_IOVEC ||		\
-	    iov_iter_type(start) == ITER_KVEC)			\
-	for (iter = (start);					\
-	     (iter).count &&					\
-	     ((iov = iov_iter_iovec(&(iter))), 1);		\
-	     iov_iter_advance(&(iter), (iov).iov_len))
-
 size_t iov_iter_copy_from_user_atomic(struct page *page,
 		struct iov_iter *i, unsigned long offset, size_t bytes);
 void iov_iter_advance(struct iov_iter *i, size_t bytes);
-- 
2.14.0.rc0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
