Re: svn commit: r25137 - in trunk/subversion: include include/private libsvn_fs libsvn_fs_base libsvn_fs_base/bdb libsvn_fs_fs libsvn_subr

["D.J. Heap" <dj...@gmail.com>]

On 5/25/07, Malcolm Rowe <ma...@farside.org.uk> wrote:
[snip]
> > --- trunk/subversion/libsvn_fs/fs-loader.c    (original)
> > +++ trunk/subversion/libsvn_fs/fs-loader.c    Thu May 24 17:09:42 2007
> > @@ -131,7 +154,34 @@
> >                               _("Failed to load module for FS type '%s'"),
> >                               fst->fs_type);
> >
> > -  SVN_ERR(initfunc(my_version, vtable));
> > +  {
> > +    apr_status_t status;
> > +    svn_error_t *err;
> > +    svn_error_t *err2;
> > +
> > +  /* Per our API compatibility rules, we cannot ensure that
> > +     svn_fs_initialize is called by the application.  If not, we
> > +     cannot create the common pool and lock in a thread-safe fashion,
> > +     nor can we clean up the common pool if libsvn_fs is dynamically
> > +     unloaded.  This function makes a best effort by creating the
> > +     common pool as a child of the global pool; the window of failure
> > +     due to thread collision is small. */
> > +  if (!common_pool)
> > +    SVN_ERR(svn_fs_initialize(NULL));
>
> Weird indentation.


Oops, thanks.


>
> > @@ -352,20 +370,36 @@
> >
> >    /* Perform the actual creation. */
> >    *fs_p = svn_fs_new(fs_config, pool);
> > -  SVN_ERR(vtable->create(*fs_p, path, pool));
> > -  return serialized_init(*fs_p, pool);
> > +  SVN_ERR(acquire_fs_mutex());
> > +  err = vtable->create(*fs_p, path, pool, common_pool);
> > +  err2 = release_fs_mutex();
> > +  if (err2)
> > +  {
> > +    svn_error_clear(err);
> > +    return err2;
> > +  }
> > +  return err;
> >  }
>
> Here and elsewhere you prioritise reporting the later mutex error over
> the earlier operation error - I think it might be better the other way
> round.
>


Yes, I wasn't sure which would be more important to return -- they
both indicate something is in pretty bad shape and it seemed like the
mutex error would be far less likely to occur.

But I have no strong attachment to either way, so I'll change it.

Thanks for the review!

DJ

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