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

Re: [PATCH] Comment consistencies in subversion/include/*.h

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-03-11 22:44:35 CET

Brian W. Fitzpatrick wrote:
> NULL, and Greg (correctly) pointed out that it should always be assumed
> that an arg cannot be NULL unless noted otherwise.

I agree that it is right to not mention this in general.

> Index: subversion/include/svn_test.h
> ===================================================================
> --- subversion/include/svn_test.h (revision 13380)
> +++ subversion/include/svn_test.h (working copy)
> @@ -98,7 +98,7 @@
> *
> * @a editor_name is a name for the editor, a string that will be
> * prepended to the editor output as shown below. @a editor_name may
> - * be the empty string, but it may not be null.
> + * be the empty string.

In cases like this, where the "not null" is useful for clarification
(contrasted with an empty string), it is probably better to put it in parentheses:

    * prepended to the editor output as shown below. @a editor_name may
  - * be the empty string, but it may not be null.
  + * be the empty string (but it may not be null).

> Index: subversion/include/svn_string.h
> ===================================================================
> --- subversion/include/svn_string.h (revision 13380)
> +++ subversion/include/svn_string.h (working copy)
> @@ -51,14 +51,6 @@
> * which means that @c strlen(s->data) does not have to equal @c
> * s->len. The null terminator is provided to make it easier to
> * pass @c s->data to C string interfaces.
> - *
> - *
> - * 2. Non-null input:
> - *
> - * All the functions below assume their input data is non-null,
> - * unless otherwise documented, and may seg fault if passed
> - * null. The input data may *contain* null bytes, of course, just
> - * the data pointer itself must not be null.
> */

That comment was useful for clarification, though a bit over the top in
verbosity. -0.5 on removing the whole of it. +0 on removing "and may seg
fault if passed null". +0.5 on prefixing a phrase like "As with other
Subversion interfaces,".

> @@ -295,8 +287,6 @@
> *
> * If @a chop_whitespace is true, then remove leading and trailing
> * whitespace from the returned strings.
> - *
> - * @a input may not be null.

+1 on removing that.

> */
> apr_array_header_t *svn_cstring_split (const char *input,
> const char *sep_chars,
> Index: subversion/include/svn_io.h
> ===================================================================
> --- subversion/include/svn_io.h (revision 13380)
> +++ subversion/include/svn_io.h (working copy)
> @@ -84,8 +84,6 @@
> * then the @c APR_DELONCLOSE flag will be used when opening the file. The
> * @c APR_BUFFERED flag will always be used.
> *
> - * @a suffix may not be null.
> - *

+1 on removing that.

> * The first attempt will just append @a suffix. If the result is not
> * a unique name, then subsequent attempts will append a dot,
> * followed by an iteration number ("2", then "3", and so on),
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 13380)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -2393,7 +2393,7 @@
> * @a validator_baton), will be called for each newly generated URL.
> *
> * @a adm_access is an access baton for the directory containing
> - * @a path, and must not be NULL.
> + * @a path.

+1 on removing that.

> */
> svn_error_t *
> svn_wc_relocate (const char *path,
> Index: subversion/include/svn_ra.h
> ===================================================================
> --- subversion/include/svn_ra.h (revision 13380)
> +++ subversion/include/svn_ra.h (working copy)
> @@ -497,8 +497,7 @@
> * @a update_target is an optional single path component to restrict
> * the scope of the update to just that entry (in the directory
> * represented by the @a session's URL). If @a update_target is the
> - * empty string, the entire directory is updated. @a update_target
> - * may not be NULL.
> + * empty string, the entire directory is updated.

That was useful for clarification, because of the text "@a update_target is an
optional ...". -1 on just removing it.

> *
> * If @a recurse is true and the target is a directory, update
> * recursively; otherwise, update just the target and its immediate

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Mar 11 22:45:50 2005

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.