Re: More documentation for errors and error handling

[ snip ]

>> + Resources associated with pools are:
>> + * Memory
>> + * Files
>
> You can't just say "Files". You would have to say which files exactly,
> and in what way they would be "freed". E.g. "files opened with
apr_blah_blah
> and svn_io_blah_blah will be closed; temporary files opened with
> svn_whatever will be removed".

Ok, I took Gregs and your comments and changed the patch. I also took the
liberty of movig the README.errors file into HACKING. Could you have a look at
the additions to the section Exception handling? Thanks!

If nobody objects, I'll commit this patch in a few days.

bye,

Erik.

Log:
[[[
Add / update documentation for errors and error handling.

* subversion/libsvn_subr/README.errors
  Removed.

* HACKING
  Add section about writing error message text.
  Move subversion/libsvn_subr/README.errors to new
  section 'Exception handling'
]]]

Index: subversion/libsvn_subr/README.errors
===================================================================
--- subversion/libsvn_subr/README.errors (revision 7945)
+++ subversion/libsvn_subr/README.errors (working copy)
@@ -1,72 +0,0 @@
-OK, here's how to use exceptions in Subversion (latest draft).
-
-
-1. Exceptions are stored in svn_error_t structures:
-
-typedef struct svn_error
-{
- ap_status_t apr_err; /* APR error value, possibly SVN_ custom err
*/
- int src_err; /* native error code (e.g. errno, h_errno...)
*/
- const char *message; /* details from producer of error */
- struct svn_error *child; /* ptr to the error we "wrap" */
- ap_pool_t *pool; /* place to generate message strings from */
-
-} svn_error_t;
-
-
-
-2. If you are the *original* creator of an error, you would do
- something like this:
-
- return (svn_create_error ((ap_status_t) err,
- NULL,
- "User not permitted to write file."
- NULL,
- my_pool));
-
-
- NOTICE the NULL field... indicating that this error has no child,
- i.e. it is the bottom-most error. Don't forget to do this!
-
-
-3. If you *receive* an error, you have three choices:
-
- a) Handle the error yourself. Use either your own code, or just
- call the primitive svn_handle_error(err). (This routine unwinds
- the stack and prints out messages.)
-
- b) Throw the error upwards, unmodified:
-
- error = some_routine (foo);
- if (error)
- return (error);
-
- Actually, a better way to do this would be with the SVN_ERR() macro,

- which does the same thing:
-
- SVN_ERR (some_routine (foo));
-
- c) Throw the error upwards, wrapping it in a new error structure
- by including it as the "child" argument:
-
- error = some_routine (foo);
- if (error)
- {
- svn_error_t *wrapper = svn_create_error (my_errno,
- NULL,
- "Authorization
failed.",
- error,
- error->pool);
- return wrapper;
- }
-
-
- Of course, there's a convenience routine which creates a
- wrapper error with the same fields as the child, except for your
- custom message:
-
- error = some_routine (foo);
- if (error)
- {
- return (svn_quick_wrap_error (error, "Authorization failed.");
- }
Index: HACKING
===================================================================
--- HACKING (revision 7945)
+++ HACKING (working copy)
@@ -19,9 +19,11 @@
    * Secure coding guidelines
    * Document everything
    * Using page breaks
+ * Error message conventions
    * Other conventions
    * APR pool usage conventions
    * APR status codes
+ * Exception handling
    * Automated tests
    * Writing test cases before code
    * Debugging the server
@@ -371,6 +373,47 @@
 
 
 
+Error message conventions
+=========================
+
+For error messages the following conventions apply:
+
+ * Provide specific error messages only when there is information
+ to add to the general error message found in
+ subversion/include/svn_error_codes.h
+
+ * Messages start with a capital letter.
+
+ * Try keeping messages below 70 characters.
+
+ * Don't end the error message with a '.'.
+
+ * Don't include newline characters in error messages.
+
+ * Quoting information is done using single quotes ('some info').
+
+ * Don't include the name of the function where the error occurs
+ in the error message. If subversion is compiled using the
+ '--enable-maintainer-mode' configure-flag, if will provide this
+ information by itself.
+
+ * When including path or filenames in the error string, be sure
+ to quote them. (i.e. "Can't find '/path/to/repos/userfile'")
+
+ * When including path or filenames in the error string, be sure
+ to convert them using 'svn_path_local_style ()' before inclusion.
+
+ * If you want to add an explanation to the error, report it
+ followed by a colon and the explanation like this:
+ "Invalid " SVN_PROP_EXTERNALS " property on '%s': "
+ "target involves '.' or '..'"
+
+ * Suggestions or other additions can be added after a semi-colon,
+ like this:
+ "Can't write to '%s': object of same name already exists; remove "
+ "before retrying"
+
+
 Other conventions
 =================
 
@@ -561,6 +604,113 @@
 
 
 
+Exception handling
+==================
+
+OK, here's how to use exceptions in Subversion.
+
+1. Exceptions are stored in svn_error_t structures:
+
+typedef struct svn_error
+{
+ ap_status_t apr_err; /* APR error value, possibly SVN_ custom err
*/
+ int src_err; /* native error code (e.g. errno, h_errno...)
*/
+ const char *message; /* details from producer of error */
+ struct svn_error *child; /* ptr to the error we "wrap" */
+ ap_pool_t *pool; /* place to generate message strings from */
+
+} svn_error_t;
+
+2. If you are the *original* creator of an error, you would do
+ something like this:
+
+ return (svn_create_error ((ap_status_t) err,
+ NULL,
+ "User not permitted to write file");
+
+
+ NOTICE the NULL field... indicating that this error has no child,
+ i.e. it is the bottom-most error.
+
+ A section on writing error messages can be found elsewhere in this
+ document under the title 'Error message conventions'.
+
+ Subversion internally uses utf-8 to store its data. This also applies
+ to the 'message' string. APR is assumed to return its data in the
current
+ locale, so any text returned by APR needs conversion to UTF-8 before
+ inclusion in the message string.
+
+3. If you *receive* an error, you have three choices:
+ a) Handle the error yourself. Use either your own code, or just
+ call the primitive svn_handle_error(err). (This routine unwinds
+ the error stack and prints out messages.)
+
+ When your routine receives an error which it intends to ignore or
+ handle itself, be sure to clean it up using svn_error_clear(). Any
+ time such an error is not cleared constitutes a *memory leak*.
+ b) Throw the error upwards, unmodified:
+
+ error = some_routine (foo);
+ if (error)
+ return (error);
+
+ Actually, a better way to do this would be with the SVN_ERR() macro,

+ which does the same thing:
+
+ SVN_ERR (some_routine (foo));
+ c) Throw the error upwards, wrapping it in a new error structure
+ by including it as the "child" argument:
+
+ error = some_routine (foo);
+ if (error)
+ {
+ svn_error_t *wrapper = svn_create_error (my_errno,
+ error,
+ "Authorization
failed");
+ return wrapper;
+ }
+
+ Of course, there's a convenience routine which creates a
+ wrapper error with the same fields as the child, except for your
+ custom message:
+
+ error = some_routine (foo);
+ if (error)
+ {
+ return (svn_quick_wrap_error (error, "Authorization failed");
+ }
+
+ The same can (and should) be done by using the SVN_ERR_W() macro:
+
+ SVN_ERR_W (some_routine (foo),
+ "Authorization failed");
+
+ In cases (b) and (c) it is important to know that resources allocated
by
+ your routine which are associated with a pool, are automatically
cleaned
+ up when the pool is destroyed. This means that there is no need to
+ cleanup these resources before passing the error. There is therefore no
+ reason not to use the SVN_ERR() and SVN_ERR_W() macros.
+
+ Resources associated with pools are:
+ * Memory
+ * Files
+ All files opened with apr_file_open are closed at pool
+ cleanup. Subversion uses this function in its svn_io_file_* api,
+ which means that files opened with svn_io_file_* or apr_file_open
+ will be closed at pool cleanup.
+
+ Some files (lock files for example) need to be removed when an
+ operation is finished. APR has the APR_DELONCLOSE flag for
+ this purpose. The following functions create files which are
+ removed on pool cleanup:
+ - apr_file_open and svn_io_file_open (when passed the
+ APR_DELONCLOSE flag)
+ - svn_io_open_unique_file (when passed TRUE in its delete_on_close)
+
+ Locked files are unlocked if they were locked using
svn_io_file_lock.
+
+
+
 Automated tests
 ===============
 

-- 
+++ GMX - die erste Adresse für Mail, Message, More +++
Neu: Preissenkung für MMS und FreeMMS! http://www.gmx.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org