Bert Huijben wrote:
> svn_ra_session_open4 already uses some variables from the config without
> duplicating them to the internal pool, so in that way it is not a first.
(It's svn_ra_open4.) That's undocumented. That sounds like a bug. Since it's
already released, we should update the doc string to say so, but going forward
do we want to continue that practice? I would have thought it better (less
surprising) not to do so.
> svn_ra_dup_session() documents that it will re-use the original arguments
> of the first Ra session, which includes references to things like batons,
> that can't be duplicated.
That's not how I interpret what it currently says:
/**
* Open a new ra session @a *new_session to the same repository as an existing
* ra session @a old_session, copying the callbacks, auth baton, etc. from the
* old session.
Please make that doc string more explicit if the intent is that the original
'config' and other pointers must still be valid.
Also svn_ra_open4() should document the lifetime requirements of each of its
arguments, and for the compound arguments ('callbacks', 'callback_baton',
'config') whether the structures themselves or only (specified) items within
them need to live for the duration of the session.
> (And Ra serf already did some internal session duplication before this
> patch)
>
> No, it is not the cleanest way to implement this, but I don't see any other
> option of creating helper Ra sessions from the Ra layer for the compact shims.
>
> The way we use it now from libsvn_client is safe as we always construct these
> sessions based on the state in svn_client_ctx_t, which must outlive both Ra
> sessions.
>
> Bert
p.s. Please avoid top-posting and HTML on this list.
Thanks.
- Julian
>From: Julian Foad
>> URL: http://svn.apache.org/r1552324
>[...]
>> @@ -494,6 +494,7 @@ svn_ra_serf__open(svn_ra_session_t *sess
>>
>> serf_sess = apr_pcalloc(pool, sizeof(*serf_sess));
>> serf_sess->pool = svn_pool_create(pool);
>> + serf_sess->config = config;
>
> Is this safe -- storing the old pointer to 'config' ...
> ... and then using it later? I don't see any lifetime guarantee.
