Date: Fri, 03 Aug 2001 21:53:10 +0100 From: Ian Dowse <iedowse@maths.tcd.ie> To: Dima Dorfman <dima@unixfreak.org> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org, mb@imp.ch Subject: Re: cvs commit: src/lib/libc/rpc rpcb_clnt.c Message-ID: <200108032153.aa03222@salmon.maths.tcd.ie> In-Reply-To: Your message of "Fri, 03 Aug 2001 12:38:22 PDT." <20010803193827.202443E28@bazooka.unixfreak.org>
next in thread | previous in thread | raw e-mail | index | archive | help
In message <20010803193827.202443E28@bazooka.unixfreak.org>, Dima Dorfman write s: >I think the first part is wrong. Here's the relevant chunk: > + if (parms.r_addr != NULL && parms.r_addr != nullstring) > + free(parms.r_addr); Yes, this definitely caused problems, as witnessed by the rpc.umntall crashes that have been seen recently. Have you tried revision 1.4 of rpcb_clnt.c which I committed yesterday? I believe this may resolve the problem you describe here. The old code (before the r1.3 change that you refer to) didn't cause any free() errors, but I think it did have a memory leak. The code in add_cache does strdup the uaddr string ad_cache->ac_uaddr = uaddr ? strdup(uaddr) : NULL; so there were two cases for getclnthandle(): 1) The entry is found in the cache, and the uaddr from the cache entry itself is returned in *targaddr, so it should not be freed by the caller. 2) The entry is not found in the cache, so tmpaddr is allocated, and a copy of this gets added to the cache (the add_cache call you quoted). The problem with this is that sometimes (case 2) the caller must free the uaddr, and other times (case 1) it must not. The original (pre-r1.3) code in __rpcb_findaddr() never freed the uaddr, so case 2 caused a memory leak. Revision 1.3 made __rpcb_findaddr always free the uaddr, which broke case 1, causing the problems in rpc.umntall. Revision 1.4, which I just committed yesterday causes getclnthandle() to always return a malloc'd uaddr string, so the caller is responsible for freeing it. This should resolve the problem that revision 1.3 caused. Does this seem the right solution, and does it fix the ypxfr problem you saw? I may not be making a lot of sense since I've just had a few pints :-) Ian To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi? <200108032153.aa03222>