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

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

From: Erik Huelsmann <ehuels_at_gmail.com>
Date: 2005-10-02 11:59:10 CEST

>
> > * subversion/include/svn_io.h,
> > subversion/libsvn_subr/io.c (svn_io_file_move): New. Function to move
> > files from one path to another, even across device boundaries. Abstracted
> > from the change to libsvn_wc/update_editor.c:install_file in r16293.
>
> Some of that information should be in the header file, not the log
> message.

Ok. Done.

> >
> > * subversion/libsvn_wc/update_editor.c (install_file): Change to use
> > svn_io_file_move().
> >
> >
> > Modified: trunk/subversion/include/svn_io.h
> > Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/include/svn_io.h?rev=16329&p1=trunk/subversion/include/svn_io.h&p2=trunk/subversion/include/svn_io.h&r1=16328&r2=16329
> > ==============================================================================
> > --- trunk/subversion/include/svn_io.h (original)
> > +++ trunk/subversion/include/svn_io.h Wed Sep 28 15:16:46 2005
> > @@ -899,6 +899,14 @@
> > apr_pool_t *pool);
> >
> >
> > +/** Move the file from @a from_path to @a to_path.
> > + * Overwrite @a to_path if it exists.
>
> How is one supposed to choose whether to use svn_io_file_move rather
> than svn_io_file_rename?

I added a note to the docstring of svn_io_file_move explaining the difference.

> > + err = svn_io_copy_file (from_path, tmp_to_path, TRUE, pool);
> > + if (err)
> > + goto failed_tmp;
> > +
> > + 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?

> > +#endif
> > +
> > + err = svn_io_remove_file (from_path, pool);
> > + if (! err)
> > + return SVN_NO_ERROR;
> > +
> > + failed_final:
>
> On non-Win32 systems failed_final is defined but not used, I get a gcc
> warning.

dlr fixed that one.

> > +#ifdef WIN32
> > + svn_error_clear (svn_io_set_file_read_write (to_path, FALSE, pool));
> > +#endif
> > + svn_error_clear (svn_io_remove_file (to_path, pool));
> > +
> > + return err;
> > +
> > + failed_tmp:
> > +#ifdef WIN32
> > + svn_error_clear (svn_io_set_file_read_write (tmp_to_path, FALSE, pool));
> > +#endif
> > + svn_error_clear (svn_io_remove_file (tmp_to_path, pool));
> > + }
> > +
> > + return err;
> > +}
> > +

bye,

Erik.
Received on Sun Oct 2 11:59:50 2005

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.