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

Re: svn commit: rev 1919 - trunk/subversion/clients/cmdline

From: Justin Erenkrantz <jerenkrantz_at_apache.org>
Date: 2002-05-10 01:25:41 CEST

On Thu, May 09, 2002 at 06:07:56PM -0500, philip@tigris.org wrote:
> Modified: trunk/subversion/clients/cmdline/util.c
> ==============================================================================
> --- trunk/subversion/clients/cmdline/util.c (original)
> +++ trunk/subversion/clients/cmdline/util.c Thu May 9 18:07:50 2002
> @@ -370,33 +370,30 @@
> const char *cmd;
> apr_file_t *tmp_file;
> svn_stringbuf_t *tmpfile_name;
> - apr_status_t apr_err;
> + apr_status_t apr_err, apr_err2;
> apr_size_t written;
> apr_finfo_t finfo_before, finfo_after;
> svn_error_t *err = SVN_NO_ERROR;
> + int sys_err;
>
> /* Try to find an editor in the environment. */
> editor = getenv ("SVN_EDITOR");
> if (! editor)
> - editor = getenv ("EDITOR");
> - if (! editor)
> editor = getenv ("VISUAL");
> + if (! editor)
> + editor = getenv ("EDITOR");
>
> - /* If there is no editor specified, return an error stating that a
> - log message should be supplied via command-line options. */
> + /* Abort if there is no editor specified */
> if (! editor)
> return svn_error_create
> (SVN_ERR_CL_NO_EXTERNAL_EDITOR, 0, NULL, pool,
> - "Could not find an external text editor in the usual environment "
> - "variables;\n"
> - "searched SVN_EDITOR, EDITOR, VISUAL, in that order. If you set\n"
> - "one of them, check if you also need to `export' it.\n");
> + "None of the environment variables "
> + "SVN_EDITOR, VISUAL or EDITOR is set.");

s/is/are/

> @@ -408,92 +405,76 @@
> apr_err = apr_file_write_full (tmp_file, contents->data,
> contents->len, &written);
>
> - /* Close the file. */
> - apr_file_close (tmp_file);
> + apr_err2 = apr_file_close (tmp_file);
> + if (APR_STATUS_IS_SUCCESS(apr_err))
> + apr_err = apr_err2;

You can just do apr_err == APR_SUCCESS. We've defined APR_SUCCESS
to always be 0. So, you could even do if (!apr_err). (In fact,
that's the construct used in the next line.)

> /* Make sure the whole CONTENTS were written, else return an error. */
> if (apr_err || (written != contents->len))
> {
> - err = svn_error_create
> + err = svn_error_createf
> (apr_err ? apr_err : SVN_ERR_INCOMPLETE_DATA, 0, NULL, pool,
> - "Unable to write initial contents to temporary file.");
> + "failed writing '%s'", tmpfile_name->data);
> goto cleanup;
> }
>
> /* Get information about the temporary file before the user has
> been allowed to edit its contents. */
> - apr_stat (&finfo_before, tmpfile_name->data,
> - APR_FINFO_MTIME | APR_FINFO_SIZE, pool);
> + apr_err = apr_stat (&finfo_before, tmpfile_name->data,
> + APR_FINFO_MTIME | APR_FINFO_SIZE, pool);
> + if (! APR_STATUS_IS_SUCCESS(apr_err))
> + {
> + err = svn_error_createf (apr_err, 0, NULL, pool,
> + "failed to stat '%s'", tmpfile_name->data);
> + goto cleanup;
> + }

Again, I'd say to just use apr_err != APR_SUCCESS or !apr_err.

<snip, snip>

> /* Get information about the temporary file after the assumed editing. */
> - apr_stat (&finfo_after, tmpfile_name->data,
> - APR_FINFO_MTIME | APR_FINFO_SIZE, pool);
> + apr_err = apr_stat (&finfo_after, tmpfile_name->data,
> + APR_FINFO_MTIME | APR_FINFO_SIZE, pool);
> + if (! APR_STATUS_IS_SUCCESS(apr_err))
> + {
> + err = svn_error_createf (apr_err, 0, NULL, pool,
> + "failed to stat '%s'", tmpfile_name->data);
> + goto cleanup;
> + }

See above. -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri May 10 01:26:42 2002

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.