[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1757761 - /subversion/trunk/subversion/libsvn_client/conflicts.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Fri, 26 Aug 2016 09:53:42 +0200

> -----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 -
> /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.

        Bert
Received on 2016-08-26 09:53:54 CEST

This is an archived mail posted to the Subversion Dev mailing list.