[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r15290 - trunk/subversion/libsvn_repos

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2005-07-07 23:34:54 CEST

sussman@tigris.org writes:

> Author: sussman
> Date: Thu Jul 7 14:25:59 2005
> New Revision: 15290
>
> Modified:
> trunk/subversion/libsvn_repos/fs-wrap.c
>
> Log:
> Hooks bugfix: if lock exists and --force not passed, don't call 'pre-(un)lock'.
>
> * subversion/libsvn_repos/fs-wrap.c
> (svn_repos_fs_lock, svn_repos_fs_unlock): if --force wasn't passed
> and a lock already exists, then bail out early; don't bother to
> call 'pre-lock', 'pre-unlock', or the fs (un)locking functions.
>
>
> Modified: trunk/subversion/libsvn_repos/fs-wrap.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_repos/fs-wrap.c?rev=15290&p1=trunk/subversion/libsvn_repos/fs-wrap.c&p2=trunk/subversion/libsvn_repos/fs-wrap.c&r1=15289&r2=15290
> ==============================================================================
> --- trunk/subversion/libsvn_repos/fs-wrap.c (original)
> +++ trunk/subversion/libsvn_repos/fs-wrap.c Thu Jul 7 14:25:59 2005
> @@ -440,6 +440,17 @@
> (SVN_ERR_FS_NO_USER, NULL,
> "Cannot lock path '%s', no authenticated username available.", path);
>
> + if (! steal_lock)
> + {
> + svn_lock_t *existing_lock;
> + SVN_ERR (svn_fs_get_lock (&existing_lock, repos->fs, path, pool));
> + if (existing_lock)
> + return svn_error_createf
> + (SVN_ERR_FS_PATH_ALREADY_LOCKED, NULL,
> + _("Lock failed: path '%s' is currently locked by user '%s'."),
> + path, existing_lock->owner);
> + }

That's a bit of a race. It's possible for the path to be unlocked
when svn_fs_get_lock is called, for a lock to created shortly
afterwards, and thus for the subsequent svn_fs_lock call to fail.

I don't suppose it matters a great deal if the error gets returned by
svn_fs_lock instead of svn_repos_fs_lock, but the fact that the race
exists means that the code is suspicious. It's a bit like libsvn_wc
doing stat() to check a file exists before open() rather than simply
handling the open() error.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jul 7 23:35:54 2005

This is an archived mail posted to the Subversion Dev mailing list.