> On Jun 6, 2022, at 9:05 PM, NeilBrown <neilb@xxxxxxx> wrote:
>
> On Mon, 06 Jun 2022, Chuck Lever wrote:
>> Both the ::iov_len field and the third parameter of memcpy() and
>> memmove() are size_t. There's no reason for the implicit conversion
>> from size_t to int and back. Change the type of @shift to make the
>> code easier to read and understand.
>
> I wouldn't object to "shift" being renamed "frag1bytes" to make the
> connection with xdr_get_next_encode_buffer() more blatant..
I thought of that. Readers would wonder why frag1bytes in
xdr_commit_encode() was a size_t, but in xdr_get_next_encode_buffer()
it's a signed int. It started to become a longer string to pull.
Maybe it's worth it?
> Reviewed-by: NeilBrown <neilb@xxxxxxx>
>
> Thanks,
> NeilBrown
>
>
>>
>> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
>> ---
>> net/sunrpc/xdr.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
>> index 08a85375b311..de8f71468637 100644
>> --- a/net/sunrpc/xdr.c
>> +++ b/net/sunrpc/xdr.c
>> @@ -933,14 +933,16 @@ EXPORT_SYMBOL_GPL(xdr_init_encode);
>> */
>> inline void xdr_commit_encode(struct xdr_stream *xdr)
>> {
>> - int shift = xdr->scratch.iov_len;
>> + size_t shift = xdr->scratch.iov_len;
>> void *page;
>>
>> if (shift == 0)
>> return;
>> +
>> page = page_address(*xdr->page_ptr);
>> memcpy(xdr->scratch.iov_base, page, shift);
>> memmove(page, page + shift, (void *)xdr->p - page);
>> +
>> xdr_reset_scratch_buffer(xdr);
>> }
>> EXPORT_SYMBOL_GPL(xdr_commit_encode);
>>
>>
>>
--
Chuck Lever
