Re: [PATCH] cmdline/commit-cmd.c

Daniel Stenberg <daniel@haxx.se> writes:

> This patch adds proper return code checks all over, it adds proper return
> codes all over (using the also included new client error code include file
> "client_errors.h"). I think most, if not all, reviewers' remarks have been
> corrected or adjusted. It is also optmized and now strips multiple SVN: lines
> for each memmove().

I don't think your line shifting algorithm is as neat as mine :-)
However I won't object to it. The problem I have with this patch is
that I don't understand how the error macros work, see below.

>
> I've made a minor effort to make the message_from_editor() function into one
> that could possibly become a more generic function for $EDITOR usage in the
> future. It will still need adjusting before that, but it is prepared.
>
> The actual running of the editor is now done using svn_io_run_cmd().
>
> This code does no further effort to translate newlines than before, and thus
> it is still likely to fail on win32. I still don't know how to deal with this
> matter.
>
> As usual, comment away! ;-)
>
> --- a Fri Feb 1 07:56:13 2002
> +++ client_errors.h Wed Jan 30 23:44:46 2002
> @@ -0,0 +1,77 @@
> +/*
> + * client_errors.h: error codes this command line client features
> + *
> + * ====================================================================
> + * Copyright (c) 2000-2002 CollabNet. All rights reserved.
> + *
> + * This software is licensed as described in the file COPYING, which
> + * you should have received as part of this distribution. The terms
> + * are also available at http://subversion.tigris.org/license-1.html.
> + * If newer versions of this license are posted there, you may use a
> + * newer version instead, at your option.
> + *
> + * This software consists of voluntary contributions made by many
> + * individuals. For exact contribution history, see the revision
> + * history and logs, available at http://subversion.tigris.org/.
> + * ====================================================================
> + */
> +
> +/* ==================================================================== */
> +
> +
> +
> +#ifndef SVN_CLIENT_ERRORS_H
> +#define SVN_CLIENT_ERRORS_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif /* __cplusplus */
> +
> +#define SVN_ERROR_START \
> + typedef enum svn_client_errno_t { \
> + SVN_ERR_CDMLINE__WARNING = SVN_ERR_LAST + 1,
> +#define SVN_ERRDEF(n, s) n,

This always throws away the string. So where is the string when the
error reporting mechanism wants to print it? Where is the equivalent
of SVN_ERROR_BUILD_ARRAY in svn_error_codes.h? Where is the
equivalent of

#define SVN_ERROR_BUILD_ARRAY
#include "client_errors.h"

in svn_error.c? Have you caused any of these errors to be generated?
Does the client print the error string when the error is reported?

> +#define SVN_ERROR_END SVN_ERR_CMDLINE__ERR_LAST } svn_client_errno_t;
> +
> +#define SVN_ERROR_ENUM_DEFINED
> +
> +/* Define custom command line client error numbers */
> +
> +SVN_ERROR_START
> +
> + /* BEGIN Client errors */
> +
> + SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_WRITING_TO_TEMPORARY_FILE,
> + "Failed writing to temporary file.")
> +
> + SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_STAT_ON_TEMPORARY_FILE,
> + "Failed getting info about temporary file.")
> +
> + SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_OPENING_TEMPORARY_FILE,
> + "Failed opening temporary file.")
> +
> + SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_READING_TEMPORARY_FILE,
> + "Failed reading temporary file.")
> +
> + /* END Client errors */
> +
> +
> +SVN_ERROR_END
> +
> +#undef SVN_ERROR_START
> +#undef SVN_ERRDEF
> +#undef SVN_ERROR_END
> +
> +#ifdef __cplusplus
> +}
> +#endif /* __cplusplus */
> +
> +
> +
> +#endif /* SVN_CLIENT_ERRORS_H */
> +
> +/* ----------------------------------------------------------------
> + * local variables:
> + * eval: (load-file "../../svn-dev.el")
> + * end:
> + */

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