[sheepdog] [PATCH v2] using xmalloc() to allocate sd_inode instead of using memory on stack

[Liu Yuan]

On Thu, Mar 06, 2014 at 01:59:23PM +0800, Robin Dong wrote:
> 'struct sd_inode' may be very large, so putting it on stack is not safe.
> 
> Signed-off-by: Robin Dong <sanbai at taobao.com>
> ---
> v1-->v2:
>   1. change size of xmalloc() to SD_INODE_HEADER_SIZE in vid_to_name_tag()
> 
>  dog/common.c    | 15 ++++++++-------
>  dog/vdi.c       | 17 +++++++++--------
>  sheep/http/kv.c |  8 +++++---
>  3 files changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/dog/common.c b/dog/common.c
> index 4be1afd..13eabd2 100644
> --- a/dog/common.c
> +++ b/dog/common.c
> @@ -132,7 +132,7 @@ int parse_vdi(vdi_parser_func_t func, size_t size, void *data)
>  {
>  	int ret;
>  	unsigned long nr;
> -	static struct sd_inode i;
> +	struct sd_inode *i = xmalloc(sizeof(*i));
>  	struct sd_req req;
>  	struct sd_rsp *rsp = (struct sd_rsp *)&req;
>  	static DECLARE_BITMAP(vdi_inuse, SD_NR_VDIS);
> @@ -156,20 +156,20 @@ int parse_vdi(vdi_parser_func_t func, size_t size, void *data)
>  		oid = vid_to_vdi_oid(nr);
>  
>  		/* for B-tree inode, we also need sd_index_header */
> -		ret = dog_read_object(oid, &i, SD_INODE_HEADER_SIZE +
> +		ret = dog_read_object(oid, i, SD_INODE_HEADER_SIZE +
>  				      sizeof(struct sd_index_header), 0, true);
>  		if (ret != SD_RES_SUCCESS) {
>  			sd_err("Failed to read inode header");
>  			continue;
>  		}
>  
> -		if (i.name[0] == '\0') /* this VDI has been deleted */
> +		if (i->name[0] == '\0') /* this VDI has been deleted */
>  			continue;
>  
>  		if (size > SD_INODE_HEADER_SIZE) {
> -			rlen = sd_inode_get_meta_size(&i, size);
> +			rlen = sd_inode_get_meta_size(i, size);
>  			ret = dog_read_object(oid,
> -					((char *)&i) + SD_INODE_HEADER_SIZE,
> +					((char *)i) + SD_INODE_HEADER_SIZE,
>  					      rlen, SD_INODE_HEADER_SIZE, true);
>  
>  			if (ret != SD_RES_SUCCESS) {
> @@ -178,11 +178,12 @@ int parse_vdi(vdi_parser_func_t func, size_t size, void *data)
>  			}
>  		}
>  
> -		snapid = vdi_is_snapshot(&i) ? i.snap_id : 0;
> -		func(i.vdi_id, i.name, i.tag, snapid, 0, &i, data);
> +		snapid = vdi_is_snapshot(i) ? i->snap_id : 0;
> +		func(i->vdi_id, i->name, i->tag, snapid, 0, i, data);
>  	}
>  
>  out:
> +	free(i);
>  	return ret;
>  }
>  
> diff --git a/dog/vdi.c b/dog/vdi.c
> index 09a87b2..999d4be 100644
> --- a/dog/vdi.c
> +++ b/dog/vdi.c
> @@ -2153,18 +2153,19 @@ out:
>  
>  static int vid_to_name_tag(uint32_t vid, char *name, char *tag)
>  {
> -	struct sd_inode inode;
> +	struct sd_inode *inode = xmalloc(SD_INODE_HEADER_SIZE);
>  	int ret;
>  
> -	ret = dog_read_object(vid_to_vdi_oid(vid), &inode, SD_INODE_HEADER_SIZE,
> -			     0, true);
> +	ret = dog_read_object(vid_to_vdi_oid(vid), inode, SD_INODE_HEADER_SIZE,
> +			      0, true);
>  	if (ret != SD_RES_SUCCESS)
> -		return ret;
> -
> -	pstrcpy(name, SD_MAX_VDI_LEN, inode.name);
> -	pstrcpy(tag, SD_MAX_VDI_TAG_LEN, inode.tag);
> +		goto out;
>  
> -	return SD_RES_SUCCESS;
> +	pstrcpy(name, SD_MAX_VDI_LEN, inode->name);
> +	pstrcpy(tag, SD_MAX_VDI_TAG_LEN, inode->tag);
> +out:
> +	free(inode);
> +	return ret;
>  }
>  
>  static int vdi_cache_info(int argc, char **argv)
> diff --git a/sheep/http/kv.c b/sheep/http/kv.c
> index 6f648e3..9ac328c 100644
> --- a/sheep/http/kv.c
> +++ b/sheep/http/kv.c
> @@ -586,7 +586,7 @@ out:
>  
>  int kv_iterate_bucket(const char *account, bucket_iter_cb cb, void *opaque)
>  {
> -	struct sd_inode account_inode;
> +	struct sd_inode *account_inode;
>  	struct bucket_iterater_arg arg = {opaque, cb, 0, 0, 0};
>  	uint32_t account_vid;
>  	uint64_t oid;
> @@ -598,18 +598,20 @@ int kv_iterate_bucket(const char *account, bucket_iter_cb cb, void *opaque)
>  		return ret;
>  	}
>  
> +	account_inode = xmalloc(sizeof(*account_inode));
>  	oid = vid_to_vdi_oid(account_vid);
>  	sys->cdrv->lock(account_vid);
> -	ret = sd_read_object(oid, (char *)&account_inode,
> +	ret = sd_read_object(oid, (char *)account_inode,
>  			     sizeof(struct sd_inode), 0);
>  	if (ret != SD_RES_SUCCESS) {
>  		sd_err("Failed to read account inode header %s", account);
>  		goto out;
>  	}
>  
> -	sd_inode_index_walk(&account_inode, bucket_iterater, &arg);
> +	sd_inode_index_walk(account_inode, bucket_iterater, &arg);
>  out:
>  	sys->cdrv->unlock(account_vid);
> +	free(account_inode);
>  	return ret;
>  }
>  
> -- 
> 1.7.12.4
> 
> -- 
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> http://lists.wpkg.org/mailman/listinfo/sheepdog

Applied thanks

Yuan