[sheepdog] [PATCH] zk: use panic instead of assert for code logic

MORITA Kazutaka morita.kazutaka at lab.ntt.co.jp
Wed May 29 10:59:35 CEST 2013


At Wed, 29 May 2013 13:57:34 +0800,
Liu Yuan wrote:
> 
> If NDEBUG is defined, the code in assert() won't be called at all.
> 
> Signed-off-by: Liu Yuan <namei.unix at gmail.com>
> ---
>  sheep/cluster/zookeeper.c |   10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/sheep/cluster/zookeeper.c b/sheep/cluster/zookeeper.c
> index 1cd39f7..e2d5cea 100644
> --- a/sheep/cluster/zookeeper.c
> +++ b/sheep/cluster/zookeeper.c
> @@ -189,7 +189,7 @@ zk_create_seq_node(const char *path, const char *value, int valuelen,
>  		panic("failed, path:%s, %s", path, zerror(rc));
>  }
>  
> -static inline ZOOAPI int zk_get_data(const char *path, void *buffer,
> +static inline ZOOAPI void zk_get_data(const char *path, void *buffer,
>  				     int *buffer_len)
>  {
>  	int rc;
> @@ -197,9 +197,8 @@ static inline ZOOAPI int zk_get_data(const char *path, void *buffer,
>  		rc = zoo_get(zhandle, path, 1, (char *)buffer,
>  			     buffer_len, NULL);
>  		if (rc != ZOK)
> -			sd_eprintf("failed, path:%s, %s", path, zerror(rc));
> +			panic("failed, path:%s, %s", path, zerror(rc));
>  	} while (rc == ZOPERATIONTIMEOUT || rc == ZCONNECTIONLOSS);
> -	return rc;
>  }

Can you wait with this change?  I have another patch which fixes
zk_create_seq_node() so that it can retry zoo_create on error cases.
The patch conflicts with this.

> @@ -475,7 +474,8 @@ static int add_join_event(void *msg, size_t msg_len)
>  	struct zk_event ev;
>  	size_t len = msg_len + sizeof(struct sd_node) * SD_MAX_NODES;
>  
> -	assert(len <= SD_MAX_EVENT_BUF_SIZE);
> +	if (len > SD_MAX_EVENT_BUF_SIZE);
> +		panic("message length too big");

There is a redundant ';' at the end of the 'if' line.

Anyway I think we should use assert() in this case.  The maximum
message size is never bigger than SD_MAX_EVENT_BUF_SIZE.  The assert()
is used only for confirmation of code's correctness.

Thanks,

Kazutaka



More information about the sheepdog mailing list