use of svn_pool_ functions

[Asbjørn Pettersen <as...@gmail.com>]

I'm still newbie to subversion code, but i have started to read the code.

I'm a little confused by the use of the svn_pool_create() and
svn_pool_destroy() functions.
The svn_pool_destroy() isn't always called !?  especially on "subpools".
The SVN_ERR() also return without calling destroy()


Example:
subversion/subversion/libsvn_client/checkout.c

svn_client__checkout_internal()
{
// ....
  apr_pool_t *session_pool = svn_pool_create(pool);

    /* Get the RA connection. */
    SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
                                             &session_url, url,
                                             peg_revision, revision, ctx,
                                             session_pool));

    SVN_ERR(svn_ra_check_path(ra_session, "", revnum, &kind, pool));
    if (kind == svn_node_none)
// ------------- Why not a call destroy() here ?
//     svn_pool_destroy(session_pool);
      return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
                               _("URL '%s' doesn't exist"), session_url);
    else if (kind == svn_node_file)
// ------------- Why not a call destroy() here ?
//     svn_pool_destroy(session_pool);
      return svn_error_createf
        (SVN_ERR_UNSUPPORTED_FEATURE , NULL,
         _("URL '%s' refers to a file, not a directory"), session_url);

    /* Get the repos UUID and root URL. */
    SVN_ERR(svn_ra_get_uuid(ra_session, &uuid, pool));
    SVN_ERR(svn_ra_get_repos_root(ra_session, &repos, pool));

    SVN_ERR(svn_io_check_path(path, &kind, pool));

    /* Finished with the RA session -- close up, but not without
       copying out useful information that needs to survive.  */
    session_url = apr_pstrdup(pool, session_url);
    uuid = (uuid ? apr_pstrdup(pool, uuid) : NULL);
    repos = (repos ? apr_pstrdup(pool, repos) : NULL);
// OK
    svn_pool_destroy(session_pool);
// ...

Re: use of svn_pool_ functions

[Peter Samuelson <pe...@p12n.org>]

[Asbjørn Pettersen]
> pool = svn_pool_create(NULL);
>  err = svn_ra_initialize(pool);
>  if (err)
>    {
>      svn_handle_error2(err, stderr, FALSE, "svnsync: ");
> // no destoy here either?
>      return EXIT_FAILURE;
>    }

There is no need to clean up memory allocations when you are about to
exit the entire program.  The will do that for you about 1 millisecond
later.  The only real point of freeing memory at that stage is to
reduce log output when running with memory leak checking tools.

Re: use of svn_pool_ functions

[Asbjørn Pettersen <as...@gmail.com>]

Ok, not so critical i hope. (If the calling functions doesn't looping then)

Here is another example from svnsync's main.c:

 pool = svn_pool_create(NULL);
  err = svn_ra_initialize(pool);
  if (err)
    {
      svn_handle_error2(err, stderr, FALSE, "svnsync: ");
// no destoy here either?
      return EXIT_FAILURE;
    }
....
//
if (err)
    {
      /* Fix up stupid default error strings. */
      if (err->apr_err == SVN_ERR_CL_INSUFFICIENT_ARGS)
        {
          svn_error_clear(err);
          err = svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, NULL,
                                 _("Not enough arguments provided; "
                                   "try 'svnsync help' for more info"));
        }
      svn_handle_error2(err, stderr, FALSE, "svnsync: ");
      svn_error_clear(err);
// no destoy here
      return EXIT_FAILURE;
    }

.....


On 10/13/06, Peter Lundblad <pl...@google.com> wrote:
>
> Asbjørn Pettersen writes:
> > I'm a little confused by the use of the svn_pool_create() and
> > svn_pool_destroy() functions.
> > The svn_pool_destroy() isn't always called !?  especially on "subpools".
> > The SVN_ERR() also return without calling destroy()
> >
> >
> > Example:
> > subversion/subversion/libsvn_client/checkout.c
> >
> > svn_client__checkout_internal()
> > {
> > // ....
> >   apr_pool_t *session_pool = svn_pool_create(pool);
> >
> >     /* Get the RA connection. */
> >     SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
> >                                              &session_url, url,
> >                                              peg_revision, revision,
> ctx,
> >                                              session_pool));
> >
> >     SVN_ERR(svn_ra_check_path(ra_session, "", revnum, &kind, pool));
> >     if (kind == svn_node_none)
> > // ------------- Why not a call destroy() here ?
> > //     svn_pool_destroy(session_pool);
>
> Because the parent pool will destroy the subpool eventually, and in the
> error case, this is expected to happen quite soon. This avoids code
> cluttering a lot.  There's some information of pool usage in hacking.
>
> Regards,
> //Peter
>

Re: use of svn_pool_ functions

[Peter Lundblad <pl...@google.com>]

Asbjørn Pettersen writes:
> I'm a little confused by the use of the svn_pool_create() and
> svn_pool_destroy() functions.
> The svn_pool_destroy() isn't always called !?  especially on "subpools".
> The SVN_ERR() also return without calling destroy()
> 
> 
> Example:
> subversion/subversion/libsvn_client/checkout.c
> 
> svn_client__checkout_internal()
> {
> // ....
>   apr_pool_t *session_pool = svn_pool_create(pool);
> 
>     /* Get the RA connection. */
>     SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
>                                              &session_url, url,
>                                              peg_revision, revision, ctx,
>                                              session_pool));
> 
>     SVN_ERR(svn_ra_check_path(ra_session, "", revnum, &kind, pool));
>     if (kind == svn_node_none)
> // ------------- Why not a call destroy() here ?
> //     svn_pool_destroy(session_pool);

Because the parent pool will destroy the subpool eventually, and in the
error case, this is expected to happen quite soon. This avoids code
cluttering a lot.  There's some information of pool usage in hacking.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org