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

Re: [PATCH] libsvn-fs-util: Convert error related functions as macros

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2007-06-04 12:45:35 CEST

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

This is an archived mail posted to the Subversion Dev mailing list.