Re: svn commit: rev 834 - trunk/subversion/libsvn_wc

sussman@tigris.org writes:

> Modified: trunk/subversion/libsvn_wc/diff.c
> ==============================================================================
> --- OLD/trunk/subversion/libsvn_wc/diff.c Thu Jan 10 23:05:56 2002
> +++ NEW/trunk/subversion/libsvn_wc/diff.c Thu Jan 10 23:05:56 2002
[snip]
> @@ -725,9 +793,73 @@
> }
> else
> {
> - /* Compare with the working copy. */
> - err = diff_cmd (temp_file_path, b->path, b->path,
> - b->edit_baton->diff_cmd_baton);
> + enum svn_wc__eol_style style;
> + const char *eol;
> +
> + SVN_ERR (svn_wc__get_eol_style (&style, &eol,
> + b->wc_path->data, b->pool));
> +
> + if ((style == svn_wc__eol_style_none)
> + || (style == svn_wc__eol_style_fixed))
> + {
> + /* Because text-base and working file use the same eol
> + style, we can compare them directly. */
> + err = diff_cmd (temp_file_path, b->path, b->path,
> + b->edit_baton->diff_cmd_baton);
> + }
> + else if (style == svn_wc__eol_style_native)
> + {
> + /* Because text-base and working file do not use the same eol
> + style, our only recourse is to convert one of them
> + before comparing. */
> + svn_stringbuf_t *tmp_dir, *tmp_workingfile;
> + apr_file_t *ignored;
> + apr_status_t apr_err;
> +
> + svn_path_split (b->wc_path, &tmp_dir, &tmp_workingfile,
> + svn_path_local_style, b->pool);
> +
> + tmp_workingfile = svn_wc__adm_path (tmp_dir, 1, b->pool,
> + tmp_workingfile, NULL);
> + SVN_ERR (svn_io_open_unique_file (&ignored,
> + &tmp_workingfile,
> + tmp_workingfile,
> + SVN_WC__TMP_EXT,
> + FALSE,
> + b->pool));
> +
> + /* Toss return value from this, it doesn't matter, we're not
> + writing to this handle anyway. */
> + apr_file_close (ignored);
> +
> + SVN_ERR (svn_io_copy_and_translate (b->path->data,
> + tmp_workingfile->data,
> + SVN_WC__DEFAULT_EOL_MARKER,
> + 0,
> + "",
> + "",
> + "",
> + "",
> + 0,
> + b->pool));

If svn_io_copy_and_translate() returns an error neither temporary file
(that's temp_file_path and tmp_workingfile) will get removed. The pool
cleanup handler that does temp_file_path elsewhere in the code has to
be removed before running diff_cmd (see the comments earlier in the
file). Please arrange for tmp_workingfile to be deleted, and either
ensure that the pool clean handler is still installed or delete
temp_file_path.

> +
> + err = diff_cmd (temp_file_path, tmp_workingfile, b->path,
> + b->edit_baton->diff_cmd_baton);

Here, diff_cmd carefully saves it's error to ensure that temporary
files can be deleted. Reinstalling the pool cleanup handler is an
option at this point.

> +
> + apr_err = apr_file_remove (tmp_workingfile->data, b->pool);
> + if (apr_err)
> + return svn_error_createf
> + (apr_err, 0, NULL, b->pool,
> + "close_file: error removing scratch file %s.",
> + tmp_workingfile->data);

This return path also fails to remove temp_file_path. Also diff_cmd
may have produced an error first, but it is not being reported. Is
this reporting of a later error intentional?

> + }
> + else
> + {
> + return svn_error_createf
> + (SVN_ERR_IO_INCONSISTENT_EOL, 0, NULL, b->pool,
> + "close_file: %s has unknown eol style property",
> + b->wc_path->data);

This return path also fails to remove temp_file_path.

Previously the diff command always removed its temp files irrespective
of whether it was reporting an error.

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