On 26 August 2016 at 10:53, Bert Huijben <bert_at_qqmail.nl> wrote:
>> -----Original Message-----
>> From: ivan_at_apache.org [mailto:ivan_at_apache.org]
>> Sent: vrijdag 26 augustus 2016 00:08
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1757761 -
>> Author: ivan
>> Date: Thu Aug 25 22:08:19 2016
>> New Revision: 1757761
>> URL: http://svn.apache.org/viewvc?rev=1757761&view=rev
>> Simplify tree conflict resolution code a bit.
>> * subversion/libsvn_client/conflicts.c
>> (resolve_update_incoming_added_file_replace): Pass NULL as FILE to
>> svn_io_open_uniquely_named() -- in this case it will close temporary file
>> Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
>> --- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
>> +++ subversion/trunk/subversion/libsvn_client/conflicts.c Thu Aug 25 22:08:19
>> @@ -5402,7 +5402,6 @@ resolve_update_incoming_added_file_repla
>> const char *lock_abspath;
>> svn_client_ctx_t *ctx = conflict->ctx;
>> svn_error_t *err;
>> - apr_file_t *backup_file;
>> const char *backup_path;
>> option_id = svn_client_conflict_option_get_id(option);
>> @@ -5422,7 +5421,7 @@ resolve_update_incoming_added_file_repla
>> * which means it does not exist in the repository. So it's a good idea
>> * to keep a backup, just in case someone picks this option by accident.
>> * First, reserve a name in the filesystem. */
>> - err = svn_io_open_uniquely_named(&backup_file, &backup_path,
>> + err = svn_io_open_uniquely_named(NULL, &backup_path,
>> @@ -5433,13 +5432,9 @@ resolve_update_incoming_added_file_repla
>> if (err)
>> goto unlock_wc;
>> - /* Close and remove the file. We're going to move the conflict victim
>> - * on top and, at least on Windows, open files can't be replaced.
>> + /* Remove the file. We're going to move the conflict victim on top and, at
>> + * least on Windows, open files can't be replaced.
>> * The WC is locked so anything racing us here is external to SVN. */
>> - err = svn_io_file_close(backup_file, scratch_pool);
>> - if (err)
>> - goto unlock_wc;
>> err = svn_error_compose_create(err, svn_io_remove_file2(backup_path,
> This remove file right after the change will not always work if the file is not
> explicitly closed before. On Windows it depends on apr opening the file
> with delete share flags (10% perf hit in some cases) and nobody else opening
> the file.
> In general strictly closing before deleting is a safer procedure.
Hi Bert, thanks for review!
I'm aware about Windows behavior of deleting opened file, but it's not
that case, because svn_io_open_uniquely_named() *explicitly* closes
file @a file is NULL:
* Open a new file (for reading and writing) with a unique name based on
* utf-8 encoded @a filename, in the directory @a dirpath. The file handle is
* returned in @a *file, and the name, which ends with @a suffix, is returned
* in @a *unique_name, also utf8-encoded. Either @a file or @a unique_name
* may be @c NULL. If @a file is @c NULL, the file will be created but not
Received on 2016-08-26 11:58:02 CEST