On Tue, 07 Jun 2022, Chuck Lever III wrote:
>
> > 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?
>
Probably worth it. Not necessarily worth it today. I have no strong
preference.
NeilBrown
>
> > 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
>
>
>
>
