On 5/25/07, Malcolm Rowe <malcolm-svn-dev@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
Received on Fri May 25 15:06:08 2007