Return-Path: Received: from userp1040.oracle.com ([156.151.31.81]:39095 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752145AbdBATsq (ORCPT ); Wed, 1 Feb 2017 14:48:46 -0500 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\)) Subject: Re: [PATCH v2 4/7] svcrdma: Clean up backchannel send header encoding From: Chuck Lever In-Reply-To: <20170201182957.GC32532@infradead.org> Date: Wed, 1 Feb 2017 14:48:17 -0500 Cc: "J. Bruce Fields" , linux-rdma@vger.kernel.org, Linux NFS Mailing List Message-Id: References: <20170131184623.14185.35480.stgit@klimt.1015granger.net> <20170131185318.14185.90053.stgit@klimt.1015granger.net> <20170201182957.GC32532@infradead.org> To: Christoph Hellwig Sender: linux-nfs-owner@vger.kernel.org List-ID: > On Feb 1, 2017, at 1:29 PM, Christoph Hellwig wrote: > > On Tue, Jan 31, 2017 at 01:53:18PM -0500, Chuck Lever wrote: >> Replace C structure-based XDR decoding with pointer arithmetic. >> Pointer arithmetic is considered more portable. >> >> Signed-off-by: Chuck Lever >> --- >> net/sunrpc/xprtrdma/svc_rdma_backchannel.c | 17 +++++++++-------- >> 1 file changed, 9 insertions(+), 8 deletions(-) >> >> diff --git a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c >> index 288e35c..abfce04 100644 >> --- a/net/sunrpc/xprtrdma/svc_rdma_backchannel.c >> +++ b/net/sunrpc/xprtrdma/svc_rdma_backchannel.c >> @@ -200,19 +200,20 @@ static int svc_rdma_bc_sendto(struct svcxprt_rdma *rdma, >> { >> struct rpc_xprt *xprt = rqst->rq_xprt; >> struct rpcrdma_xprt *r_xprt = rpcx_to_rdmax(xprt); >> - struct rpcrdma_msg *headerp = (struct rpcrdma_msg *)rqst->rq_buffer; >> + __be32 *p; >> int rc; >> >> /* Space in the send buffer for an RPC/RDMA header is reserved >> * via xprt->tsh_size. >> */ >> + p = (__be32 *)rqst->rq_buffer; > > Why not move the assignment onto the same line as the declaration > of p? My "convention" is that if the value of an automatic variable remains constant throughout a function, it can be initialized where it is declared. Whereas the value of "headerp" is fixed, and header offsets were generated by the compiler, the value of "p" here varies as the header is marshaled. It's merely a documentation convention; a nit, really. > Also rq_buffer is a void pointer, so the cast shouldn't be needed. OK. > Otherwise looks fine: > > Reviewed-by: Christoph Hellwig > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever