On 24 September 2015 at 18:50, Stefan Sperling <s...@elego.de> wrote: > On Thu, Sep 24, 2015 at 05:40:45PM +0300, Ivan Zhakov wrote: >> On 24 September 2015 at 17:34, Bert Huijben <b...@qqmail.nl> wrote: >> >> >> >> +/* Parse a given URL_STR, fill in all supplied fields of URI >> >> + * structure. >> >> + * >> >> + * This function is a compatibility wrapper around apr_uri_parse(). >> >> + * Different apr-util versions set apr_uri_t.path to either NULL or "" >> >> + * for root paths, and serf expects to see "/". This function always >> >> + * sets URI.path to "/" for these paths. */ >> >> +svn_error_t * >> >> +svn_ra_serf__uri_parse(apr_uri_t *uri, >> >> + const char *url_str, >> >> + apr_pool_t *pool); >> > >> > I think the pool should be named result_pool here. >> > > > +1 > >> I think we use POOL name if function accepts just one pool, and >> SCRATCH_POOL/RESULT_POOL in other case. Is not it? >> >> I would not mind to rename POOL to RESULT_POOL in this particular >> case, but I'm not sure that we should use RESULT_POOL in all other >> cases if function accepts one pool. > > We certainly have functions that take only a scratch_pool. > The idea is to identify the purpose of the pool, and not only > in the case where there are 2 pools. I don't think we may use other places with only scratch_pool argument as reason: we also have many functions that accepts just POOL and use it as scratch pool. And we also have many functions that uses it as result pool.
Anyway I agree that in this particular place RESULT_POOL makes more sense so I renamed argument in r1705088. -- Ivan Zhakov
