Hi Bhuvan,
> Index: subversion/include/private/svn_fs_util.h
> ===================================================================
> --- subversion/include/private/svn_fs_util.h	(revision 25279)
> +++ subversion/include/private/svn_fs_util.h	(working copy)
> @@ -20,6 +20,7 @@
>  #ifndef SVN_FS_UTIL_H
>  #define SVN_FS_UTIL_H
>  
> +#include "svn_private_config.h"
>  #ifdef __cplusplus
>  extern "C" {
>  #endif /* __cplusplus */
> @@ -41,33 +42,62 @@
>  
>  /* SVN_ERR_FS_NOT_MUTABLE: the caller attempted to change a node
>     outside of a transaction.  */
>   
May be you can have the above comment as
 /* Attempted to change a node outside of a transaction.  */
*
*
Your comment should try to explain the type of args passed to Macro.
**
> -svn_error_t *svn_fs__err_not_mutable(svn_fs_t *fs, svn_revnum_t rev,
> -                                        const char *path);
> +#define SVN_FS__ERR_NOT_MUTABLE(f, r, p)                               \
> +  svn_error_createf                                                    \
> +  (SVN_ERR_FS_NOT_MUTABLE, 0,                                          \
> +   _("File is not mutable: filesystem '%s', revision %ld, path '%s'"), \
> +   f->path, r, p)
>  
>  /* SVN_ERR_FS_NOT_DIRECTORY: PATH does not refer to a directory in FS.  */
>   
I think you need not explain this macro as it is self evident. Your 
comment should try to explain the type of args passed to Macro.
> -svn_error_t *svn_fs__err_not_directory(svn_fs_t *fs, const char *path);
> +#define SVN_FS__ERR_NOT_DIRECTORY(f, p)                \
> +  svn_error_createf                                    \
> +  (SVN_ERR_FS_NOT_DIRECTORY, 0,                        \
> +   _("'%s' is not a directory in filesystem '%s'"),    \
> +   p, f->path)
>  
>  /* SVN_ERR_FS_NOT_FILE: PATH does not refer to a file in FS.  */
>   
I think you need not explain this macro as it is self evident. Your 
comment should try to explain the type of args passed to Macro.
> -svn_error_t *svn_fs__err_not_file(svn_fs_t *fs, const char *path);
> +#define SVN_FS__ERR_NOT_FILE(f, p)                     \
> +  svn_error_createf                                    \
> +  (SVN_ERR_FS_NOT_FILE, 0,                             \
> +   _("'%s' is not a file in filesystem '%s'"),         \
> +   p, f->path) 
>  
>  /* SVN_ERR_FS_PATH_ALREADY_LOCKED: a path is already locked.  */
>   
I think you need not explain this macro as it is self evident. Your 
comment should try to explain the type of args passed to Macro.
> -svn_error_t *svn_fs__err_path_already_locked(svn_fs_t *fs,
> -                                                svn_lock_t *lock);
> +#define SVN_FS__ERR_PATH_ALREADY_LOCKED(f, l)                        \
> +  svn_error_createf                                                  \
> +  (SVN_ERR_FS_PATH_ALREADY_LOCKED, 0,                                \
> +   _("Path '%s' is already locked by user '%s' in filesystem '%s'"), \
> +   l->path, l->owner, f->path)
>  
>  /* SVN_ERR_FS_NO_SUCH_LOCK: there is no lock on PATH in FS.  */
>   
I think you need not explain this macro as it is self evident. Your 
comment should try to explain the type of args passed to Macro.
> -svn_error_t *svn_fs__err_no_such_lock(svn_fs_t *fs, const char *path);
> +#define SVN_FS__ERR_NO_SUCH_LOCK(f, p)                 \
> +  svn_error_createf                                    \
> +  (SVN_ERR_FS_NO_SUCH_LOCK, 0,                         \
> +   _("No lock on path '%s' in filesystem '%s'"),       \
> +   p, f->path)
>  
>  /* SVN_ERR_FS_LOCK_EXPIRED: TOKEN's lock in FS has been auto-expired. */
>   
I think you need not explain this macro as it is self evident. Your 
comment should try to explain the type of args passed to Macro.
> -svn_error_t *svn_fs__err_lock_expired(svn_fs_t *fs, const char *token);
> +#define SVN_FS__ERR_LOCK_EXPIRED(f, t)                         \
> +  svn_error_createf                                            \
> +  (SVN_ERR_FS_LOCK_EXPIRED, 0,                                 \
> +   _("Lock has expired:  lock-token '%s' in filesystem '%s'"), \
> +   t, f->path)
>  
>  /* SVN_ERR_FS_NO_USER: FS does not have a user associated with it. */
>   
I think you need not explain this macro as it is self evident. Your 
comment should try to explain the type of args passed to Macro.
> -svn_error_t *svn_fs__err_no_user(svn_fs_t *fs);
> +#define SVN_FS__ERR_NO_USER(f)                                    \
>   
Retain the original formal variable names.
> +  svn_error_createf                                               \
> +  (SVN_ERR_FS_NO_USER, 0,                                         \
> +   _("No username is currently associated with filesystem '%s'"), \
> +   f->path)
>  
>  /* SVN_ERR_FS_LOCK_OWNER_MISMATCH: trying to use a lock whose LOCK_OWNER
>     doesn't match the USERNAME associated with FS.  */
>   
May be you can change this to
/* SVN_FS__ERR_LOCK_OWNER_MISMATCH: trying to use a lock whose LOCK_OWNER
    doesn't match the USERNAME associated with FS.  */
  Document the type of formal arguments in your comment.
Thanks
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Jun  4 12:45:14 2007