On 24 September 2015 at 18:50, Stefan Sperling <stsp_at_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 <bert_at_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
Received on 2015-09-24 17:56:41 CEST