Branko Čibej wrote:
> Comments below.
>
> Jay Freeman (saurik) wrote:
>
>> OK, here is the end result of the various Win32 permission fixes (and
>> one NULL initialization problem):
>>
>>
>> * ./subversion/include/svn_io.h (svn_io_set_file_read_write): Added
>> prototype for a new svn_io_set_file_read_write() function that removes
>> the read-only flag from the specified file.
>>
>> * ./subversion/libsvn_wc/adm_files.c (close_adm_file): When a file is
>> overwritten by another file on Win32, it is important that the file not
>> be read-only. Before performing the rename, close_adm_file() now
>> removes the read-only flag on the destination.
>>
>> (svn_wc__remove_adm_file): On Win32, files that are marked read-only
>> cannot be deleted. Before performing the delete,
>> svn_wc__remove_adm_file now removes the read-only flag on the
>> destination.
>>
>> * ./subversion/libsvn_wc/get_editor.c (svn_wc_install_file): There was a
>> small bug where the fresh_keywords_value pointer was failed to be
>> initialized to NULL.
>>
>> * ./subversion/libsvn_subr/io.c (svn_io_set_file_read_write):
>> Implemented a new function that removes the read-only flag from the
>> specified file using apr_file_attrs_set().
>>
>>
>>
>> Index: .\subversion\include\svn_io.h
>> ===================================================================
>> --- .\subversion\include\svn_io.h
>> +++ .\subversion\include\svn_io.h Sun Feb 10 17:54:56 2002
>> @@ -144,6 +144,11 @@
>> */
>> svn_error_t *svn_io_set_file_read_only (const char *path, apr_pool_t
>> *pool);
>>
>> +/* Make a file as writable as the operating system allows.
>> + * PATH is the path to the file.
>> + */
>> +svn_error_t *svn_io_set_file_read_write (const char *path, apr_pool_t
>> *pool);
>> +
>>
>> /* Read a line from FILE into BUF, but not exceeding *LIMIT bytes.
>> * Does not include newline, instead '\0' is put there.
>> Index: .\subversion\libsvn_wc\adm_files.c
>> ===================================================================
>> --- .\subversion\libsvn_wc\adm_files.c
>> +++ .\subversion\libsvn_wc\adm_files.c Mon Feb 11 03:36:56 2002
>> @@ -645,6 +645,7 @@
>> ...)
>> {
>> apr_status_t apr_err = 0;
>> + svn_error_t *err = NULL;
>> int components_added;
>> va_list ap;
>>
>> @@ -681,6 +682,11 @@
>> v_extend_with_adm_name (tmp_path, extension, 1, pool, ap);
>> va_end (ap);
>>
>> + /* Remove read-only flag on destination. */
>> + err = svn_io_set_file_read_write (path->data, pool);
>> + if (err && !APR_STATUS_IS_ENOENT(err->apr_err))
>> + return err;
>> +
>>
> I don't like looking through the svn_error_t. Might it be better to have:
>
> svn_error_t *svn_io_set_file_read_write (const char *path,
> svn_boolean_t ignore_enoent,
> apr_pool_t *pool)
>
>
> and similarily for svn_io_set_file_read_only? Then you could use just
> SVN_ERR here.
>
>>
>> /* Rename. */
>> apr_err = apr_file_rename (tmp_path->data, path->data, pool);
>> if (APR_STATUS_IS_SUCCESS(apr_err))
>> @@ -998,6 +1004,9 @@
>> va_start (ap, pool);
>> components_added = v_extend_with_adm_name (path, NULL, 0, pool, ap);
>> va_end (ap);
>> +
>> + /* Remove read-only flag on path. */
>> + SVN_ERR(svn_io_set_file_read_write (path->data, pool));
>>
>> apr_err = apr_file_remove (path->data, pool);
>> if (apr_err)
>> Index: .\subversion\libsvn_wc\get_editor.c
>> ===================================================================
>> --- .\subversion\libsvn_wc\get_editor.c
>> +++ .\subversion\libsvn_wc\get_editor.c Mon Feb 11 03:34:56 2002
>> @@ -1524,7 +1524,7 @@
>> {
>> /* Did we get a new keywords value passed into this routine? */
>> int i;
>> - svn_stringbuf_t *fresh_keywords_value;
>> + svn_stringbuf_t *fresh_keywords_value = NULL;
>>
>> /* Rats, here's one case where it would be *nice* to have a
>> hash instead of an array. */
>> Index: .\subversion\libsvn_subr\io.c
>> ===================================================================
>> --- .\subversion\libsvn_subr\io.c
>> +++ .\subversion\libsvn_subr\io.c Mon Feb 11 03:29:12 2002
>> @@ -378,6 +378,34 @@
>> return SVN_NO_ERROR;
>> }
>>
>> +svn_error_t *
>> +svn_io_set_file_read_write (const char *path, apr_pool_t *pool)
>> +{
>> + apr_status_t status;
>> + apr_fileattrs_t attributes;
>> +
>> +#ifdef SVN__APR_FILE_ATTRS_GET
>> + /* This is what he would do if the get function was available in APR.
>> If
>> + the attribute removing stuff gets in to APR and we don't do this,
>> then
>> + removing read-only will remove executable. Since we don't ever set
>> + executable at present that is not a big problem */
>> + status = apr_file_attrs_get (path, &attributes, pool);
>> + if (!APR_STATUS_IS_SUCCESS(status))
>> + return svn_error_createf (status, 0, NULL, pool,
>> + "failed to get attributes for file '%s'",
>> path);
>> +#else
>> + attributes = 0;
>> +#endif
>>
> Blech. I think the signature for apr_file_attrs_set should change
> instead (we can do that, since presumably nobody else is using that
> function yet):
>
> apr_status_t apr_file_attrs_set (const char *path,
> apr_uint32_t attribures,
> apr_uint32_t attr_mask,
> apr_pool_t *pool);
>
>
> Here, ATTR_MASK tells which bits in ATTRIBUTES are valid.
>
> (Of course, apr_file_attrs_get should be added for symmetry anyway).
>
>
>>
>> +
>> + attributes &= !APR_FILE_ATTR_READONLY;
>>
> Nope: you want "attributes &= ~APTR_FILE_ATTR_READONLY". Well,
> actually you want the changed signature, but failing that, 1's
> complement is surely better than logical negation. :-)
/me notices similar prob in apr/file_io/win32/filestat.c
/me starts fixing that
>>
>> + status = apr_file_attrs_set (path, attributes, pool);
>> + if (!APR_STATUS_IS_SUCCESS(status))
>> + return svn_error_createf (status, 0, NULL, pool,
>> + "failed to set file '%s' read-write",
>> path);
>> +
>> + return SVN_NO_ERROR;
>> +}
>> +
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>
>
>
--
Brane Čibej <brane_at_xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:37:06 2006