[sheepdog] [PATCH v7 7/7] zookeeper: clean code by macro RETURN_IF_ERROR() and RETURN_VOID_IF_ERROR

[MORITA Kazutaka]

At Sun, 23 Jun 2013 20:05:27 -0700,
Kai Zhang wrote:
> 
> Signed-off-by: Kai Zhang <kyle at zelin.io>
> ---
>  sheep/cluster/zookeeper.c |  178 ++++++++++++++++-----------------------------
>  1 file changed, 62 insertions(+), 116 deletions(-)
> 
> diff --git a/sheep/cluster/zookeeper.c b/sheep/cluster/zookeeper.c
> index b27583f..c990eb0 100644
> --- a/sheep/cluster/zookeeper.c
> +++ b/sheep/cluster/zookeeper.c
> @@ -155,6 +155,24 @@ static struct zk_node this_node;
>  	default:							\
>  		panic("failed, path:%s, %s", path, zerror(rc));		\
>  	}
> +#define RETURN_IF_ERROR(stmt, fmt, ...)					\
> +	do {								\
> +		int __rc = stmt;					\
> +		if (__rc) {						\

Should be (__rc != ZOK).  I didn't know that ZOK is defined as zero.

> +			sd_eprintf("failed, " fmt ", %s",		\
> +				   ##__VA_ARGS__, zerror(__rc));	\
> +			return __rc;					\
> +		}							\
> +	} while (0)
> +#define RETURN_VOID_IF_ERROR(stmt, fmt, ...)				\
> +	do {								\
> +		int __rc = stmt;					\
> +		if (__rc) {						\

Should be (__rc != ZOK).

> +			sd_eprintf("failed, " fmt ", %s",		\
> +				   ##__VA_ARGS__, zerror(__rc));	\
> +			return;						\
> +		}							\
> +	} while (0)
>  
>  static inline ZOOAPI int zk_delete_node(const char *path, int version)
>  {
> @@ -285,7 +303,7 @@ static int zk_queue_peek(bool *peek)
>  		rc = ZOK;
>  		*peek = false;
>  	} else
> -		sd_eprintf("failed, %s", zerror(rc));
> +		RETURN_IF_ERROR(rc, "");

This looks like abuse of RETURN_IF_ERROR.  rc is always not ZOK here
and you use the macro to print the error message.  I think using a
switch statement is easier to read.

> @@ -626,10 +614,8 @@ static int zk_get_least_seq(const char *parent, char *least_seq_path,
>  			return ZOK;
>  		} else if (rc == ZNONODE)
>  			continue;
> -		else {
> -			sd_eprintf("failed, %s", zerror(rc));
> -			return rc;
> -		}
> +		else
> +			RETURN_IF_ERROR(rc, "");

Same here, using a switch statement looks cleaner.


> @@ -660,10 +644,8 @@ static int zk_find_master(int *master_seq, char *master_name)
>  					   "start to compete master");
>  				(*master_seq)++;
>  				continue;
> -			} else {
> -				sd_eprintf("failed, %s", zerror(rc));
> -				return rc;
> -			}
> +			} else
> +				RETURN_IF_ERROR(rc, "");

Same here.

> @@ -700,10 +680,8 @@ static int zk_verify_last_sheep_join(int seq, int *last_sheep)
>  		else if (rc == ZNONODE) {
>  			(*last_sheep)++;
>  			continue;
> -		} else {
> -			sd_eprintf("failed, %s", zerror(rc));
> -			return rc;
> -		}
> +		} else
> +			RETURN_IF_ERROR(rc, "");

Same here.

Thanks,

Kazutaka