subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Zhakov <i...@visualsvn.com>
Subject Re: svn commit: r1757761 - /subversion/trunk/subversion/libsvn_client/conflicts.c
Date Fri, 26 Aug 2016 09:57:34 GMT
On 26 August 2016 at 10:53, Bert Huijben <bert@qqmail.nl> wrote:
>> -----Original Message-----
>> From: ivan@apache.org [mailto:ivan@apache.org]
>> Sent: vrijdag 26 augustus 2016 00:08
>> To: commits@subversion.apache.org
>> Subject: svn commit: r1757761 -
>> /subversion/trunk/subversion/libsvn_client/conflicts.c
>>
>> Author: ivan
>> Date: Thu Aug 25 22:08:19 2016
>> New Revision: 1757761
>>
>> URL: http://svn.apache.org/viewvc?rev=1757761&view=rev
>> Log:
>> 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
>>    automatically.
>>
>> Modified:
>>     subversion/trunk/subversion/libsvn_client/conflicts.c
>>
>> Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflic
>> ts.c?rev=1757761&r1=1757760&r2=1757761&view=diff
>> ================================================================
>> ==============
>> --- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
>> +++ subversion/trunk/subversion/libsvn_client/conflicts.c Thu Aug 25 22:08:19
>> 2016
>> @@ -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,
>>                                     svn_dirent_dirname(local_abspath,
>>                                                        scratch_pool),
>>                                     svn_dirent_basename(local_abspath,
>> @@ -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,
>> TRUE,
>>                                                            scratch_pool));
>
> 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
 * open.
]]]

-- 
Ivan Zhakov

Mime
View raw message