Bert Huijben wrote:
-----Original Message-----
From:stef...@apache.org [mailto:stef...@apache.org]
Sent: zondag 15 april 2012 13:21
To:comm...@subversion.apache.org
Subject: svn commit: r1326307 - in /subversion/trunk: ./ build/ac-macros/
subversion/include/ subversion/include/private/ subversion/libsvn_fs_fs/
subversion/libsvn_subr/ subversion/mod_dav_svn/ subversion/svnadmin/
subversion/svnserve/ subversion/tests/ subver...
Partial review for some of the code in svn_io.h
Author: stefan2
Date: Sun Apr 15 11:20:58 2012
New Revision: 1326307
URL:http://svn.apache.org/viewvc?rev=1326307&view=rev
Log:
Merge all changes (-r1298521-1326293) from branches/revprop-cache to
trunk
and resolve minor conflicts.
Added:
subversion/trunk/subversion/include/private/svn_named_atomic.h
- copied unchanged from r1326293, subversion/branches/revprop-
cache/subversion/include/private/svn_named_atomic.h
subversion/trunk/subversion/libsvn_subr/svn_named_atomic.c
- copied unchanged from r1326293, subversion/branches/revprop-
cache/subversion/libsvn_subr/svn_named_atomic.c
subversion/trunk/subversion/tests/libsvn_subr/named_atomic-test-
common.h
- copied unchanged from r1326293, subversion/branches/revprop-
cache/subversion/tests/libsvn_subr/named_atomic-test-common.h
subversion/trunk/subversion/tests/libsvn_subr/named_atomic-test-
proc.c
- copied unchanged from r1326293, subversion/branches/revprop-
cache/subversion/tests/libsvn_subr/named_atomic-test-proc.c
subversion/trunk/subversion/tests/libsvn_subr/named_atomic-test.c
- copied, changed from r1326293, subversion/branches/revprop-
cache/subversion/tests/libsvn_subr/named_atomic-test.c
Modified:
subversion/trunk/ (props changed)
subversion/trunk/build.conf
subversion/trunk/build/ac-macros/svn-macros.m4
subversion/trunk/configure.ac
subversion/trunk/subversion/include/svn_error_codes.h
subversion/trunk/subversion/include/svn_fs.h
subversion/trunk/subversion/include/svn_io.h
subversion/trunk/subversion/libsvn_fs_fs/caching.c
subversion/trunk/subversion/libsvn_fs_fs/fs.h
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
subversion/trunk/subversion/libsvn_subr/io.c
subversion/trunk/subversion/mod_dav_svn/dav_svn.h
subversion/trunk/subversion/mod_dav_svn/mod_dav_svn.c
subversion/trunk/subversion/mod_dav_svn/repos.c
subversion/trunk/subversion/svnadmin/main.c
subversion/trunk/subversion/svnserve/main.c
subversion/trunk/subversion/svnserve/serve.c
subversion/trunk/subversion/svnserve/server.h
subversion/trunk/subversion/tests/libsvn_repos/repos-test.c
subversion/trunk/subversion/tests/libsvn_subr/ (props changed)
subversion/trunk/subversion/tests/svn_test.h
Propchange: subversion/trunk/
------------------------------------------------------------------------------
Merged /subversion/branches/revprop-cache:r1298521-1326293
Modified: subversion/trunk/subversion/include/svn_io.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.
h?rev=1326307&r1=1326306&r2=1326307&view=diff
==========================================================
====================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Sun Apr 15 11:20:58 2012
@@ -682,6 +682,39 @@ svn_io_file_lock2(const char *lock_file,
svn_boolean_t exclusive,
svn_boolean_t nonblocking,
apr_pool_t *pool);
+
+/**
+ * Lock the file @a lockfile_handle. If @a exclusive is TRUE,
+ * obtain exclusive lock, otherwise obtain shared lock.
+ *
+ * If @a nonblocking is TRUE, do not wait for the lock if it
+ * is not available: throw an error instead.
+ *
+ * Lock will be automatically released when @a pool is cleared or destroyed.
+ * You may also explicitly call @ref svn_io_unlock_open_file.
+ * Use @a pool for memory allocations. @a pool must be the pool that
+ * @a lockfile_handle has been created in or one of its sub-pools.
+ *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_io_lock_open_file(apr_file_t *lockfile_handle,
+ svn_boolean_t exclusive,
+ svn_boolean_t nonblocking,
+ apr_pool_t *pool);
What does a nonexclusive shared lock 'lock'?
What does it block? What does it allow?
Does it just block exclusive locks? Or does it block writers?
The same thing as in svn_io_file_lock2() ;)
I haven't looked at the implementation but
my guess is that it will prevent exclusive locks.
+
+/**
+ * Unlock the file @a lockfile_handle.
+ *
+ * Use @a pool for memory allocations. @a pool must be the pool that
+ * @a lockfile_handle has been created in or one of its sub-pools.
+ *
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_io_unlock_open_file(apr_file_t *lockfile_handle,
+ apr_pool_t *pool);
+
/**
* Flush any unwritten data from @a file to disk. Use @a pool for
* memory allocations.
<snip>
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.
c?rev=1326307&r1=1326306&r2=1326307&view=diff
==========================================================
====================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Sun Apr 15 11:20:58 2012
@@ -1866,29 +1866,23 @@ file_clear_locks(void *arg)
#endif
svn_error_t *
-svn_io_file_lock2(const char *lock_file,
- svn_boolean_t exclusive,
- svn_boolean_t nonblocking,
- apr_pool_t *pool)
+svn_io_lock_open_file(apr_file_t *lockfile_handle,
+ svn_boolean_t exclusive,
+ svn_boolean_t nonblocking,
+ apr_pool_t *pool)
{
int locktype = APR_FLOCK_SHARED;
- apr_file_t *lockfile_handle;
- apr_int32_t flags;
apr_status_t apr_err;
+ const char *fname;
if (exclusive)
locktype = APR_FLOCK_EXCLUSIVE;
-
- flags = APR_READ;
- if (locktype == APR_FLOCK_EXCLUSIVE)
- flags |= APR_WRITE;
-
if (nonblocking)
locktype |= APR_FLOCK_NONBLOCK;
- SVN_ERR(svn_io_file_open(&lockfile_handle, lock_file, flags,
- APR_OS_DEFAULT,
- pool));
+ /* We need this only in case of an error but this is cheap to get -
+ * so we do it here for clarity. */
+ apr_err = apr_file_name_get(&fname, lockfile_handle);
fname here is path encoded, not necessary utf-8 encoded.
/* Get lock on the filehandle. */
apr_err = apr_file_lock(lockfile_handle, locktype);
@@ -1915,11 +1909,11 @@ svn_io_file_lock2(const char *lock_file,
case APR_FLOCK_SHARED:
return svn_error_wrap_apr(apr_err,
_("Can't get shared lock on file '%s'"),
- svn_dirent_local_style(lock_file, pool));
+ svn_dirent_local_style(fname, pool));
case APR_FLOCK_EXCLUSIVE:
return svn_error_wrap_apr(apr_err,
_("Can't get exclusive lock on file
'%s'"),
- svn_dirent_local_style(lock_file, pool));
+ svn_dirent_local_style(fname, pool));
So it can't just be used in error messages like here.
default:
SVN_ERR_MALFUNCTION();
}
@@ -1936,6 +1930,58 @@ svn_io_file_lock2(const char *lock_file,
return SVN_NO_ERROR;
}
+svn_error_t *
+svn_io_unlock_open_file(apr_file_t *lockfile_handle,
+ apr_pool_t *pool)
+{
+ const char *fname;
+ apr_status_t apr_err = apr_file_unlock(lockfile_handle);
+
+ /* We need this only in case of an error but this is cheap to get -
+ * so we do it here for clarity. */
+ apr_err = apr_file_name_get(&fname, lockfile_handle);
+
+/* On Windows and OS/2 file locks are automatically released when
+ the file handle closes */
+#if !defined(WIN32)&& !defined(__OS2__)
+ apr_pool_cleanup_kill(pool, lockfile_handle, file_clear_locks);
+#endif
+
+ return apr_err
+ ? svn_error_wrap_apr(apr_err, _("Can't unlock file '%s'"),
+ svn_dirent_local_style(fname, pool))
apr_err depends just on the filename getting.
Is failing to obtain the name an unlock error?
Oops. I got the checking order wrong there.
Is a succeeded obtaining of the name a success of the unlock?
On Windows this function can just return SVN_NO_ERROR, as we don't unlock, so
the unlock can't fail. The name obtaining is unrelated. And converting locale
for the filename in the error (which can be expensive) should be added for
!Windows.
(There are file local optimized helpers for the path conversions)
+ : SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_io_file_lock2(const char *lock_file,
+ svn_boolean_t exclusive,
+ svn_boolean_t nonblocking,
+ apr_pool_t *pool)
+{
+ int locktype = APR_FLOCK_SHARED;
+ apr_file_t *lockfile_handle;
+ apr_int32_t flags;
+ apr_status_t apr_err;
+
+ if (exclusive)
+ locktype = APR_FLOCK_EXCLUSIVE;
+
+ flags = APR_READ;
+ if (locktype == APR_FLOCK_EXCLUSIVE)
+ flags |= APR_WRITE;
+
+ if (nonblocking)
+ locktype |= APR_FLOCK_NONBLOCK;
+
+ SVN_ERR(svn_io_file_open(&lockfile_handle, lock_file, flags,
+ APR_OS_DEFAULT,
+ pool));
+
+ /* Get lock on the filehandle. */
+ return svn_io_lock_open_file(lockfile_handle, exclusive, nonblocking,
pool);
+}
+
Thanks for the review! r1328939 should address these issues.
-- Stefan^2.
