Re: [PATCH] issue #532 read-only admin area

On Mon, Jan 28, 2002 at 12:24:31AM +0000, Philip Martin wrote:
>...
> * subversion/libsvn_wc/log.c
> (replace_text_base): Pass new parameter to svn_io_copy_file.
> (log_do_file_readonly): New function.
> (start_handler): Handle SVN_WC__LOG_READONLY.

I can see the need for a 'readonly' log item, to make a file readonly after
it has been created in tmp and then installed. But how about if this was
part of the 'mv' operation -- move it into place and make it readonly. That
would seem to be the /real/ operation that is being attempted, rather than a
distinct 'make readonly' operation.

>...
> +++ ../svn/subversion/libsvn_subr/io.c Sun Jan 27 22:28:57 2002
> @@ -324,15 +324,55 @@
>
>
> svn_error_t *
> -svn_io_copy_file (const char *src, const char *dst, apr_pool_t *pool)
> +svn_io_copy_file (const char *src,
> + const char *dst,
> + svn_boolean_t copy_perms,
> + apr_pool_t *pool)
> {
> apr_status_t apr_err;
> + apr_finfo_t finfo;
> +
> + /* apr_copy_file will copy the permissions, if that is not wanted we need
> + to get the initial permissions and restore them after the copy. */
> + if (!copy_perms)

I'd much rather see a patch that changes apr_copy_file() to take a
'copy_perms' flag. That is much better than the extra operations here.

> + {
> + apr_file_t *tmp;
> +
> + /* It must be possible to open APR_WRITE if the copy is going to
> + succeed. */
> + apr_err = apr_file_open (&tmp, dst,
> + APR_WRITE | APR_CREATE, APR_OS_DEFAULT,
> + pool);
> + if (! APR_STATUS_IS_SUCCESS (apr_err))
> + return svn_error_createf (apr_err, 0, NULL, pool,
> + "cannot open '%s'", dst);
> +
> + apr_err = apr_stat (&finfo, dst,
> + APR_FINFO_UPROT | APR_FINFO_GROUP | APR_FINFO_WPROT,
> + pool);
> + if (! APR_STATUS_IS_SUCCESS (apr_err))
> + return svn_error_createf (apr_err, 0, NULL, pool,
> + "cannot stat '%s'", dst);
> +
> + apr_err = apr_file_close (tmp);
> + if (! APR_STATUS_IS_SUCCESS (apr_err))
> + return svn_error_createf (apr_err, 0, NULL, pool,
> + "cannot close '%s'", dst);
> + }

And even if we *didn't* patch APR, then you don't need to open/close the
file ahead of time. Just do the apr_stat(). If the file wasn't there, then
you would simply skip the copy of the perms (and leave the new copy as
APR_OS_DEFAULT).

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org