* [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer. @ 2017-05-25 20:35 Steve Dickson 2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson 2017-05-30 16:45 ` [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson 0 siblings, 2 replies; 4+ messages in thread From: Steve Dickson @ 2017-05-25 20:35 UTC (permalink / raw) To: Linux NFS Mailing list commit 7ea36ee introduced a svc_freeargs() call that ended up freeing static pointer. It turns out the allocations for the rmt_args is not necessary . The xdr routines (xdr_bytes) will handle the memory management and the largest possible message size is UDPMSGSIZE (due to UDP only) which is smaller than RPC_BUF_MAX Signed-off-by: Steve Dickson <steved@redhat.com> --- src/rpcb_svc_com.c | 39 ++++++--------------------------------- 1 file changed, 6 insertions(+), 33 deletions(-) diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c index cb63afd..1fc2229 100644 --- a/src/rpcb_svc_com.c +++ b/src/rpcb_svc_com.c @@ -612,9 +612,9 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, struct netconfig *nconf; struct netbuf *caller; struct r_rmtcall_args a; - char *buf_alloc = NULL, *outbufp; + char *outbufp; char *outbuf_alloc = NULL; - char buf[RPC_BUF_MAX], outbuf[RPC_BUF_MAX]; + char outbuf[RPC_BUF_MAX]; struct netbuf *na = (struct netbuf *) NULL; struct rpc_msg call_msg; int outlen; @@ -635,36 +635,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, } if (si.si_socktype != SOCK_DGRAM) return; /* Only datagram type accepted */ - sendsz = __rpc_get_t_size(si.si_af, si.si_proto, UDPMSGSIZE); - if (sendsz == 0) { /* data transfer not supported */ - if (reply_type == RPCBPROC_INDIRECT) - svcerr_systemerr(transp); - return; - } - /* - * Should be multiple of 4 for XDR. - */ - sendsz = ((sendsz + 3) / 4) * 4; - if (sendsz > RPC_BUF_MAX) { -#ifdef notyet - buf_alloc = alloca(sendsz); /* not in IDR2? */ -#else - buf_alloc = malloc(sendsz); -#endif /* notyet */ - if (buf_alloc == NULL) { - if (debugging) - xlog(LOG_DEBUG, - "rpcbproc_callit_com: No Memory!\n"); - if (reply_type == RPCBPROC_INDIRECT) - svcerr_systemerr(transp); - return; - } - a.rmt_args.args = buf_alloc; - } else { - a.rmt_args.args = buf; - } + sendsz = UDPMSGSIZE; call_msg.rm_xid = 0; /* For error checking purposes */ + memset(&a, 0, sizeof(a)); /* Zero out the input buffer */ if (!svc_getargs(transp, (xdrproc_t) xdr_rmtcall_args, (char *) &a)) { if (reply_type == RPCBPROC_INDIRECT) svcerr_decode(transp); @@ -704,7 +678,8 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, if (rbl == (rpcblist_ptr)NULL) { #ifdef RPCBIND_DEBUG if (debugging) - xlog(LOG_DEBUG, "not found\n"); + xlog(LOG_DEBUG, "prog %lu vers %lu: not found\n", + a.rmt_prog, a.rmt_vers); #endif if (reply_type == RPCBPROC_INDIRECT) svcerr_noprog(transp); @@ -937,8 +912,6 @@ out: } if (local_uaddr) free(local_uaddr); - if (buf_alloc) - free(buf_alloc); if (outbuf_alloc) free(outbuf_alloc); if (na) { -- 2.9.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer 2017-05-25 20:35 [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson @ 2017-05-25 20:35 ` Steve Dickson 2017-05-30 16:45 ` Steve Dickson 2017-05-30 16:45 ` [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson 1 sibling, 1 reply; 4+ messages in thread From: Steve Dickson @ 2017-05-25 20:35 UTC (permalink / raw) To: Linux NFS Mailing list Now that sendz is a fixed size (UDPMSGSIZE) which is small then RPC_BUF_MAX, no need to check the sendz size. Signed-off-by: Steve Dickson <steved@redhat.com> --- src/rpcb_svc_com.c | 33 +++++---------------------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c index 1fc2229..d36b090 100644 --- a/src/rpcb_svc_com.c +++ b/src/rpcb_svc_com.c @@ -612,8 +612,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, struct netconfig *nconf; struct netbuf *caller; struct r_rmtcall_args a; - char *outbufp; - char *outbuf_alloc = NULL; char outbuf[RPC_BUF_MAX]; struct netbuf *na = (struct netbuf *) NULL; struct rpc_msg call_msg; @@ -674,7 +672,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, rpcbs_rmtcall(versnum - 2, reply_type, a.rmt_prog, a.rmt_vers, a.rmt_proc, transp->xp_netid, rbl); - if (rbl == (rpcblist_ptr)NULL) { #ifdef RPCBIND_DEBUG if (debugging) @@ -793,24 +790,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, call_msg.rm_call.cb_rpcvers = RPC_MSG_VERSION; call_msg.rm_call.cb_prog = a.rmt_prog; call_msg.rm_call.cb_vers = a.rmt_vers; - if (sendsz > RPC_BUF_MAX) { -#ifdef notyet - outbuf_alloc = alloca(sendsz); /* not in IDR2? */ -#else - outbuf_alloc = malloc(sendsz); -#endif /* notyet */ - if (outbuf_alloc == NULL) { - if (reply_type == RPCBPROC_INDIRECT) - svcerr_systemerr(transp); - if (debugging) - xlog(LOG_DEBUG, - "rpcbproc_callit_com: No memory!\n"); - goto error; - } - xdrmem_create(&outxdr, outbuf_alloc, sendsz, XDR_ENCODE); - } else { - xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE); - } + + memset(outbuf, '\0', sendsz); /* Zero out the output buffer */ + xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE); + if (!xdr_callhdr(&outxdr, &call_msg)) { if (reply_type == RPCBPROC_INDIRECT) svcerr_systemerr(transp); @@ -875,10 +858,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, goto error; } outlen = (int) XDR_GETPOS(&outxdr); - if (outbuf_alloc) - outbufp = outbuf_alloc; - else - outbufp = outbuf; na = uaddr2taddr(nconf, local_uaddr); if (!na) { @@ -887,7 +866,7 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, goto error; } - if (sendto(fd, outbufp, outlen, 0, (struct sockaddr *)na->buf, na->len) + if (sendto(fd, outbuf, outlen, 0, (struct sockaddr *)na->buf, na->len) != outlen) { if (debugging) xlog(LOG_DEBUG, @@ -912,8 +891,6 @@ out: } if (local_uaddr) free(local_uaddr); - if (outbuf_alloc) - free(outbuf_alloc); if (na) { free(na->buf); free(na); -- 2.9.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer 2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson @ 2017-05-30 16:45 ` Steve Dickson 0 siblings, 0 replies; 4+ messages in thread From: Steve Dickson @ 2017-05-30 16:45 UTC (permalink / raw) To: Linux NFS Mailing list On 05/25/2017 04:35 PM, Steve Dickson wrote: > Now that sendz is a fixed size (UDPMSGSIZE) which > is small then RPC_BUF_MAX, no need to check the > sendz size. > > Signed-off-by: Steve Dickson <steved@redhat.com> Committed... steved. > --- > src/rpcb_svc_com.c | 33 +++++---------------------------- > 1 file changed, 5 insertions(+), 28 deletions(-) > > diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c > index 1fc2229..d36b090 100644 > --- a/src/rpcb_svc_com.c > +++ b/src/rpcb_svc_com.c > @@ -612,8 +612,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > struct netconfig *nconf; > struct netbuf *caller; > struct r_rmtcall_args a; > - char *outbufp; > - char *outbuf_alloc = NULL; > char outbuf[RPC_BUF_MAX]; > struct netbuf *na = (struct netbuf *) NULL; > struct rpc_msg call_msg; > @@ -674,7 +672,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > > rpcbs_rmtcall(versnum - 2, reply_type, a.rmt_prog, a.rmt_vers, > a.rmt_proc, transp->xp_netid, rbl); > - > if (rbl == (rpcblist_ptr)NULL) { > #ifdef RPCBIND_DEBUG > if (debugging) > @@ -793,24 +790,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > call_msg.rm_call.cb_rpcvers = RPC_MSG_VERSION; > call_msg.rm_call.cb_prog = a.rmt_prog; > call_msg.rm_call.cb_vers = a.rmt_vers; > - if (sendsz > RPC_BUF_MAX) { > -#ifdef notyet > - outbuf_alloc = alloca(sendsz); /* not in IDR2? */ > -#else > - outbuf_alloc = malloc(sendsz); > -#endif /* notyet */ > - if (outbuf_alloc == NULL) { > - if (reply_type == RPCBPROC_INDIRECT) > - svcerr_systemerr(transp); > - if (debugging) > - xlog(LOG_DEBUG, > - "rpcbproc_callit_com: No memory!\n"); > - goto error; > - } > - xdrmem_create(&outxdr, outbuf_alloc, sendsz, XDR_ENCODE); > - } else { > - xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE); > - } > + > + memset(outbuf, '\0', sendsz); /* Zero out the output buffer */ > + xdrmem_create(&outxdr, outbuf, sendsz, XDR_ENCODE); > + > if (!xdr_callhdr(&outxdr, &call_msg)) { > if (reply_type == RPCBPROC_INDIRECT) > svcerr_systemerr(transp); > @@ -875,10 +858,6 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > goto error; > } > outlen = (int) XDR_GETPOS(&outxdr); > - if (outbuf_alloc) > - outbufp = outbuf_alloc; > - else > - outbufp = outbuf; > > na = uaddr2taddr(nconf, local_uaddr); > if (!na) { > @@ -887,7 +866,7 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > goto error; > } > > - if (sendto(fd, outbufp, outlen, 0, (struct sockaddr *)na->buf, na->len) > + if (sendto(fd, outbuf, outlen, 0, (struct sockaddr *)na->buf, na->len) > != outlen) { > if (debugging) > xlog(LOG_DEBUG, > @@ -912,8 +891,6 @@ out: > } > if (local_uaddr) > free(local_uaddr); > - if (outbuf_alloc) > - free(outbuf_alloc); > if (na) { > free(na->buf); > free(na); > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer. 2017-05-25 20:35 [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson 2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson @ 2017-05-30 16:45 ` Steve Dickson 1 sibling, 0 replies; 4+ messages in thread From: Steve Dickson @ 2017-05-30 16:45 UTC (permalink / raw) To: Linux NFS Mailing list On 05/25/2017 04:35 PM, Steve Dickson wrote: > commit 7ea36ee introduced a svc_freeargs() call > that ended up freeing static pointer. > > It turns out the allocations for the rmt_args > is not necessary . The xdr routines (xdr_bytes) will > handle the memory management and the largest > possible message size is UDPMSGSIZE (due to UDP only) > which is smaller than RPC_BUF_MAX > > Signed-off-by: Steve Dickson <steved@redhat.com> Committed.... steved. > --- > src/rpcb_svc_com.c | 39 ++++++--------------------------------- > 1 file changed, 6 insertions(+), 33 deletions(-) > > diff --git a/src/rpcb_svc_com.c b/src/rpcb_svc_com.c > index cb63afd..1fc2229 100644 > --- a/src/rpcb_svc_com.c > +++ b/src/rpcb_svc_com.c > @@ -612,9 +612,9 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > struct netconfig *nconf; > struct netbuf *caller; > struct r_rmtcall_args a; > - char *buf_alloc = NULL, *outbufp; > + char *outbufp; > char *outbuf_alloc = NULL; > - char buf[RPC_BUF_MAX], outbuf[RPC_BUF_MAX]; > + char outbuf[RPC_BUF_MAX]; > struct netbuf *na = (struct netbuf *) NULL; > struct rpc_msg call_msg; > int outlen; > @@ -635,36 +635,10 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > } > if (si.si_socktype != SOCK_DGRAM) > return; /* Only datagram type accepted */ > - sendsz = __rpc_get_t_size(si.si_af, si.si_proto, UDPMSGSIZE); > - if (sendsz == 0) { /* data transfer not supported */ > - if (reply_type == RPCBPROC_INDIRECT) > - svcerr_systemerr(transp); > - return; > - } > - /* > - * Should be multiple of 4 for XDR. > - */ > - sendsz = ((sendsz + 3) / 4) * 4; > - if (sendsz > RPC_BUF_MAX) { > -#ifdef notyet > - buf_alloc = alloca(sendsz); /* not in IDR2? */ > -#else > - buf_alloc = malloc(sendsz); > -#endif /* notyet */ > - if (buf_alloc == NULL) { > - if (debugging) > - xlog(LOG_DEBUG, > - "rpcbproc_callit_com: No Memory!\n"); > - if (reply_type == RPCBPROC_INDIRECT) > - svcerr_systemerr(transp); > - return; > - } > - a.rmt_args.args = buf_alloc; > - } else { > - a.rmt_args.args = buf; > - } > + sendsz = UDPMSGSIZE; > > call_msg.rm_xid = 0; /* For error checking purposes */ > + memset(&a, 0, sizeof(a)); /* Zero out the input buffer */ > if (!svc_getargs(transp, (xdrproc_t) xdr_rmtcall_args, (char *) &a)) { > if (reply_type == RPCBPROC_INDIRECT) > svcerr_decode(transp); > @@ -704,7 +678,8 @@ rpcbproc_callit_com(struct svc_req *rqstp, SVCXPRT *transp, > if (rbl == (rpcblist_ptr)NULL) { > #ifdef RPCBIND_DEBUG > if (debugging) > - xlog(LOG_DEBUG, "not found\n"); > + xlog(LOG_DEBUG, "prog %lu vers %lu: not found\n", > + a.rmt_prog, a.rmt_vers); > #endif > if (reply_type == RPCBPROC_INDIRECT) > svcerr_noprog(transp); > @@ -937,8 +912,6 @@ out: > } > if (local_uaddr) > free(local_uaddr); > - if (buf_alloc) > - free(buf_alloc); > if (outbuf_alloc) > free(outbuf_alloc); > if (na) { > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-30 16:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-05-25 20:35 [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson 2017-05-25 20:35 ` [PATCH 2/2] rpcbproc_callit_com: No need to allocate output buffer Steve Dickson 2017-05-30 16:45 ` Steve Dickson 2017-05-30 16:45 ` [PATCH 1/2] rpcbproc_callit_com: Stop freeing a static pointer Steve Dickson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox