You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <e....@gmx.net> on 2003/12/06 23:35:38 UTC
More documentation for errors and error handling
Thanks to the reminder of Sander Roobol, I have taken the suggestions from
you and applied them to the patch.
There is a remaining point of discussion though. Currently there are
tex-info files in doc/programmer/design documenting different concepts in Subversion
design. Error handling and pool programming are definitely at the core of
Subversion design, yet neither has documentation in that directory. Only pool
usage has a section in HACKING.
There is a document about handling errors in Subversion, which is (IMHO
mis-)located in subversion/libsvn_subr. If the section about how to write error
messages belongs in HACKING, then doesn't a section about handling errors
belong in it too?!
The patch below updates the current documentation in subversion/libsvn_subr.
Please provide any comments and additions for the patch as well as any
reactions to integrating into HACKING.
bye,
Erik.
PS: The patch below can also be found at
http://encorps.dnsalias.com/svn-public/patches/errors.log-patch
Log:
[[[
Add / update documentation for errors and error handling.
* HACKING
Add section about writing error message text.
* subversion/libsvn_subr/README.errors
Update and extend documentation about handling errors.
]]]
Index: subversion/libsvn_subr/README.errors
===================================================================
--- subversion/libsvn_subr/README.errors (revision 7945)
+++ subversion/libsvn_subr/README.errors (working copy)
@@ -1,4 +1,4 @@
-OK, here's how to use exceptions in Subversion (latest draft).
+OK, here's how to use exceptions in Subversion.
1. Exceptions are stored in svn_error_t structures:
@@ -20,7 +20,7 @@
return (svn_create_error ((ap_status_t) err,
NULL,
- "User not permitted to write file."
+ "User not permitted to write file"
NULL,
my_pool));
@@ -32,9 +32,13 @@
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.)
+ 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 are not cleared constitutes a *memory leak*.
+
b) Throw the error upwards, unmodified:
error = some_routine (foo);
@@ -45,7 +49,7 @@
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:
@@ -54,7 +58,7 @@
{
svn_error_t *wrapper = svn_create_error (my_errno,
NULL,
- "Authorization
failed.",
+ "Authorization failed",
error,
error->pool);
return wrapper;
@@ -68,5 +72,20 @@
error = some_routine (foo);
if (error)
{
- return (svn_quick_wrap_error (error, "Authorization failed.");
+ 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
Index: HACKING
===================================================================
--- HACKING (revision 7945)
+++ HACKING (working copy)
@@ -19,6 +19,7 @@
* Secure coding guidelines
* Document everything
* Using page breaks
+ * Error message conventions
* Other conventions
* APR pool usage conventions
* APR status codes
@@ -371,6 +372,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
=================
--
+++ 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
Re: More documentation for errors and error handling
Posted by Erik Huelsmann <e....@gmx.net>.
> A few more points mentioned below, mostly trivial; otherwise +1.
Some more fine-tuning done, to include your additions and the fact that Greg
Hudson introduced a convention that the message text should start at a new
line.
Committed in r7956.
Erik.
--
+++ 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
Re: More documentation for errors and error handling
Posted by Julian Foad <ju...@btopenworld.com>.
Erik Huelsmann wrote:
>
> 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.
A few more points mentioned below, mostly trivial; otherwise +1.
- Julian
> Index: HACKING
> ===================================================================
> --- HACKING (revision 7945)
> +++ HACKING (working copy)
> @@ -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 */
apr_status_t
> + int src_err; /* native error code (e.g. errno, h_errno...) */
That is not a member of svn_error.
> + 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 */
There are also "file" and "line" members.
> +
> +} 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,
return (svn_create_error (SVN_ERR_FOO,
> + 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
UTF-8
> + 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.
Since blank lines are used to separate paragraphs, it would be nice also to have blank lines between subsections:
> +3. If you *receive* an error, you have three choices:
blank line
> + 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*.
blank line
> + b) Throw the error upwards, unmodified:
> +
> + error = some_routine (foo);
[...]
> + which does the same thing:
> +
> + SVN_ERR (some_routine (foo));
blank line
> + c) Throw the error upwards, wrapping it in a new error structure
> + by including it as the "child" argument:
> +
> + error = some_routine (foo);
[...]
> + Resources associated with pools are:
blank line
> + * Memory
blank line
> + * Files
> + All files opened with apr_file_open are closed at pool
[...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: More documentation for errors and error handling
Posted by Erik Huelsmann <e....@gmx.net>.
[ 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
Re: More documentation for errors and error handling
Posted by Julian Foad <ju...@btopenworld.com>.
Erik Huelsmann wrote:
[...]
> Index: subversion/libsvn_subr/README.errors
> ===================================================================
> --- subversion/libsvn_subr/README.errors (revision 7945)
> +++ subversion/libsvn_subr/README.errors (working copy)
[...]
> + 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 are not cleared constitutes a *memory leak*.
"such an error _is_ not ..."
[...]
> + 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".
Are error messages meant to be composed in UTF-8 (and the error handling mechanism then converts them to local encoding just before printing them)? I think this should be stated somewhere.
Everything else sounds good to me.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: More documentation for errors and error handling
Posted by Greg Hudson <gh...@MIT.EDU>.
This all seems fine. But:
On Sat, 2003-12-06 at 18:35, Erik Huelsmann wrote:
> Index: subversion/libsvn_subr/README.errors
Some of the examples here need to have pool arguments removed.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org