Re: svn commit: r16329 - in trunk/subversion: libsvn_subr libsvn_wc

Erik Huelsmann <ehuels@gmail.com> writes:

>> > + err = svn_io_file_rename (tmp_to_path, to_path, pool);
>> > + if (err)
>> > + goto failed_tmp;
>> > +
>> > +#ifdef WIN32
>> > + /* Windows won't let us delete a file marked as read-only,
>> > + so, mark as read+write */
>> > + err = svn_io_set_file_read_write (from_path, FALSE, pool);
>> > + if (err)
>> > + goto failed_final;
>>
>> The error handling is a bit inconsistent, if the above set_read_write
>> fails then the following remove_file is skipped, but for the
>> failed_final and failed_tmp cases the remove_file is always attempted.
>
> I hadn't thought of them as inconsistent: The former is part of the
> rename-or-copy codepath (leaving behind the from_path when it fails),
> whereas the latter is in the cleanup path trying to clean up any
> intermediate files as hard as it can.
>
> Does it need changing, as far as you're concerned?

On closer inspection I think that the WIN32 stuff should be removed
from this function since svn_io_remove_file already handles the
svn_io_set_file_read_write.

>
>> > +#endif
>> > +
>> > + err = svn_io_remove_file (from_path, pool);
>> > + if (! err)
>> > + return SVN_NO_ERROR;
>> > +

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org