* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.