[patch] Use the appropriate naming convention for protected APIs

[Daniel Rall <dl...@collab.net>]

On Thu, 21 Sep 2006, David James wrote:

> On 9/21/06, Daniel Rall <dl...@collab.net> wrote:
> >Okay, I'm committing a change which makes an APR_VERSION_AT_LEAST()
> >macro available.
> >
> >How about the svn_hash_clear() function I committed to
> >svn_hash.h/hash.c a couple days ago?  It's basically just a wrapper
> >around or re-implementation of APR's apr_hash_clear() API (also found
> >in APR 1.3.0).  Should I then do away with svn_hash_clear(), and
> >conditionally define apr_hash_clear() in svn_compat.h?
> 
> That seems nice and consistent. In that case, it might make sense to
> rename 'svn_hash_clear' to 'svn__hash_clear', so as to make clear that
> it's a private function which shouldn't be called directly (except
> through the macro).

How's this?  We might want to do the same to the
svn_atomic_init_once() API in svn_atomic.h...

[[[
Rename svn_hash_clear() to svn_hash__clear() to reflect that it's not
part of Subversion's public API.

* subversion/include/svn_hash.h
* subversion/libsvn_subr/hash.c
  (svn_hash__clear): Rename from svn_hash_clear().

* subversion/libsvn_client/repos_diff.c
  (close_directory): Update for new API name.
]]]

Index: subversion/include/svn_hash.h
===================================================================
--- subversion/include/svn_hash.h	(revision 21640)
+++ subversion/include/svn_hash.h	(working copy)
@@ -200,7 +200,7 @@
  *
  * @since New in 1.5.
  */
-svn_error_t *svn_hash_clear(apr_hash_t *hash);
+svn_error_t *svn_hash__clear(apr_hash_t *hash);
 
 /** @} */
 
Index: subversion/libsvn_subr/hash.c
===================================================================
--- subversion/libsvn_subr/hash.c	(revision 21640)
+++ subversion/libsvn_subr/hash.c	(working copy)
@@ -436,7 +436,7 @@
 /*** Misc. hash APIs ***/
 
 svn_error_t *
-svn_hash_clear(apr_hash_t *hash)
+svn_hash__clear(apr_hash_t *hash)
 {
 #if APR_VERSION_AT_LEAST(1, 3, 0)
   apr_hash_clear(hash);
Index: subversion/libsvn_client/repos_diff.c
===================================================================
--- subversion/libsvn_client/repos_diff.c	(revision 21640)
+++ subversion/libsvn_client/repos_diff.c	(working copy)
@@ -794,7 +794,7 @@
   svn_error_t *err;
 
   if (eb->dry_run)
-    svn_hash_clear(svn_client__dry_run_deletions(eb->diff_cmd_baton));
+    svn_hash__clear(svn_client__dry_run_deletions(eb->diff_cmd_baton));
 
   if (b->propchanges->nelts > 0)
     {

Re: [patch] Use the appropriate naming convention for protected APIs

[Kamesh Jayachandran <ka...@collab.net>]

Looks fine with me. Posted a similar patch for svn_atomic_init_once.

With regards
Kamesh Jayachandran
Daniel Rall wrote:
> On Thu, 21 Sep 2006, David James wrote:
>
>   
>> On 9/21/06, Daniel Rall <dl...@collab.net> wrote:
>>     
>>> Okay, I'm committing a change which makes an APR_VERSION_AT_LEAST()
>>> macro available.
>>>
>>> How about the svn_hash_clear() function I committed to
>>> svn_hash.h/hash.c a couple days ago?  It's basically just a wrapper
>>> around or re-implementation of APR's apr_hash_clear() API (also found
>>> in APR 1.3.0).  Should I then do away with svn_hash_clear(), and
>>> conditionally define apr_hash_clear() in svn_compat.h?
>>>       
>> That seems nice and consistent. In that case, it might make sense to
>> rename 'svn_hash_clear' to 'svn__hash_clear', so as to make clear that
>> it's a private function which shouldn't be called directly (except
>> through the macro).
>>     
>
> How's this?  We might want to do the same to the
> svn_atomic_init_once() API in svn_atomic.h...
>
> [[[
> Rename svn_hash_clear() to svn_hash__clear() to reflect that it's not
> part of Subversion's public API.
>
> * subversion/include/svn_hash.h
> * subversion/libsvn_subr/hash.c
>   (svn_hash__clear): Rename from svn_hash_clear().
>
> * subversion/libsvn_client/repos_diff.c
>   (close_directory): Update for new API name.
> ]]]
>
> Index: subversion/include/svn_hash.h
> ===================================================================
> --- subversion/include/svn_hash.h	(revision 21640)
> +++ subversion/include/svn_hash.h	(working copy)
> @@ -200,7 +200,7 @@
>   *
>   * @since New in 1.5.
>   */
> -svn_error_t *svn_hash_clear(apr_hash_t *hash);
> +svn_error_t *svn_hash__clear(apr_hash_t *hash);
>  
>  /** @} */
>  
> Index: subversion/libsvn_subr/hash.c
> ===================================================================
> --- subversion/libsvn_subr/hash.c	(revision 21640)
> +++ subversion/libsvn_subr/hash.c	(working copy)
> @@ -436,7 +436,7 @@
>  /*** Misc. hash APIs ***/
>  
>  svn_error_t *
> -svn_hash_clear(apr_hash_t *hash)
> +svn_hash__clear(apr_hash_t *hash)
>  {
>  #if APR_VERSION_AT_LEAST(1, 3, 0)
>    apr_hash_clear(hash);
> Index: subversion/libsvn_client/repos_diff.c
> ===================================================================
> --- subversion/libsvn_client/repos_diff.c	(revision 21640)
> +++ subversion/libsvn_client/repos_diff.c	(working copy)
> @@ -794,7 +794,7 @@
>    svn_error_t *err;
>  
>    if (eb->dry_run)
> -    svn_hash_clear(svn_client__dry_run_deletions(eb->diff_cmd_baton));
> +    svn_hash__clear(svn_client__dry_run_deletions(eb->diff_cmd_baton));
>  
>    if (b->propchanges->nelts > 0)
>      {
>
>   

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