On Fri, 2010-04-23, Stefan Sperling wrote: > On Fri, Apr 23, 2010 at 01:01:24PM -0000, [email protected] wrote: > > Author: stsp > > Date: Fri Apr 23 13:01:23 2010 > > New Revision: 937275 > > > > URL: http://svn.apache.org/viewvc?rev=937275&view=rev > > Log: > > Pool usage fixes in the translation stream. > > > > Do not maintain a private pool for each translation stream, > > because the user cannot control unbound growth of this "secret" pool. > > Instead, rely on callers to provide pools with sufficient lifetime > > when the stream is created. "make check" agrees. > > > > Suggested by: gstein > > > > * subversion/include/svn_subst.h > > (svn_subst_stream_translated): Rename POOL argument to RESULT_POOL. > > Document pool lifetime requirements for EOL_STR and KEYWORDS parameters. > > It is unclear whether changing pool lifetime expectations of this API > is acceptable. This problem was known before commit. Greg wanted to > comment on this post-commit.
I'll comment: > > --- subversion/trunk/subversion/include/svn_subst.h (original) > > +++ subversion/trunk/subversion/include/svn_subst.h Fri Apr 23 13:01:23 2010 > > @@ -301,7 +301,9 @@ svn_subst_translate_stream(svn_stream_t > > * if @a repair is @c TRUE, convert any line ending to @a eol_str. > > * Recognized line endings are: "\n", "\r", and "\r\n". > > * > > - * The stream returned is allocated in @a pool. > > + * The stream returned is allocated in @a result_pool. > > + * @a eol_str and @a keywords are expected to be allocated in a pool > > + * with sufficient lifetime for use by the stream. It seems to me that adding this requirement now is unsafe, because our API users would not expect this. I think it would be best to revert that part of the change, restoring the code that copies EOL_STR and KEYWORDS into what is now RESULT_POOL. - Julian

