> -----Original Message-----
> From: stefan2_at_apache.org [mailto:stefan2_at_apache.org]
> Sent: zondag 15 april 2012 13:21
> To: commits_at_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?
> +
> +/**
> + * 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?
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);
> +}
> +
>
>
Bert
Received on 2012-04-16 00:13:44 CEST