linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] pnfsblock: fix return code confusion
@ 2011-09-09 16:33 Jim Rees
  2011-09-10  7:29 ` Benny Halevy
  0 siblings, 1 reply; 2+ messages in thread
From: Jim Rees @ 2011-09-09 16:33 UTC (permalink / raw)
  To: Benny Halevy; +Cc: linux-nfs, peter honeyman

Always return PTR_ERR, not NULL, from nfs4_blk_get_deviceinfo and
nfs4_blk_decode_device.

Check for IS_ERR, not NULL, in bl_set_layoutdriver when calling
nfs4_blk_get_deviceinfo.

Signed-off-by: Jim Rees <rees@umich.edu>
---
 fs/nfs/blocklayout/blocklayout.c    |   20 ++++++++++++--------
 fs/nfs/blocklayout/blocklayoutdev.c |   13 ++++++++-----
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
index 9143e61..7debb55 100644
--- a/fs/nfs/blocklayout/blocklayout.c
+++ b/fs/nfs/blocklayout/blocklayout.c
@@ -794,7 +794,7 @@ nfs4_blk_get_deviceinfo(struct nfs_server *server, const struct nfs_fh *fh,
 			struct nfs4_deviceid *d_id)
 {
 	struct pnfs_device *dev;
-	struct pnfs_block_dev *rv = NULL;
+	struct pnfs_block_dev *rv;
 	u32 max_resp_sz;
 	int max_pages;
 	struct page **pages = NULL;
@@ -812,18 +812,20 @@ nfs4_blk_get_deviceinfo(struct nfs_server *server, const struct nfs_fh *fh,
 	dev = kmalloc(sizeof(*dev), GFP_NOFS);
 	if (!dev) {
 		dprintk("%s kmalloc failed\n", __func__);
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	}
 
 	pages = kzalloc(max_pages * sizeof(struct page *), GFP_NOFS);
 	if (pages == NULL) {
 		kfree(dev);
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 	}
 	for (i = 0; i < max_pages; i++) {
 		pages[i] = alloc_page(GFP_NOFS);
-		if (!pages[i])
+		if (!pages[i]) {
+			rv = ERR_PTR(-ENOMEM);
 			goto out_free;
+		}
 	}
 
 	memcpy(&dev->dev_id, d_id, sizeof(*d_id));
@@ -836,8 +838,10 @@ nfs4_blk_get_deviceinfo(struct nfs_server *server, const struct nfs_fh *fh,
 	dprintk("%s: dev_id: %s\n", __func__, dev->dev_id.data);
 	rc = nfs4_proc_getdeviceinfo(server, dev);
 	dprintk("%s getdevice info returns %d\n", __func__, rc);
-	if (rc)
+	if (rc) {
+		rv = ERR_PTR(rc);
 		goto out_free;
+	}
 
 	rv = nfs4_blk_decode_device(server, dev);
  out_free:
@@ -855,7 +859,7 @@ bl_set_layoutdriver(struct nfs_server *server, const struct nfs_fh *fh)
 	struct pnfs_devicelist *dlist = NULL;
 	struct pnfs_block_dev *bdev;
 	LIST_HEAD(block_disklist);
-	int status = 0, i;
+	int status, i;
 
 	dprintk("%s enter\n", __func__);
 
@@ -887,8 +891,8 @@ bl_set_layoutdriver(struct nfs_server *server, const struct nfs_fh *fh)
 		for (i = 0; i < dlist->num_devs; i++) {
 			bdev = nfs4_blk_get_deviceinfo(server, fh,
 						       &dlist->dev_id[i]);
-			if (!bdev) {
-				status = -ENODEV;
+			if (IS_ERR(bdev)) {
+				status = PTR_ERR(bdev);
 				goto out_error;
 			}
 			spin_lock(&b_mt_id->bm_lock);
diff --git a/fs/nfs/blocklayout/blocklayoutdev.c b/fs/nfs/blocklayout/blocklayoutdev.c
index 46a2ce6..d08ba91 100644
--- a/fs/nfs/blocklayout/blocklayoutdev.c
+++ b/fs/nfs/blocklayout/blocklayoutdev.c
@@ -109,7 +109,7 @@ struct pnfs_block_dev *
 nfs4_blk_decode_device(struct nfs_server *server,
 		       struct pnfs_device *dev)
 {
-	struct pnfs_block_dev *rv = NULL;
+	struct pnfs_block_dev *rv;
 	struct block_device *bd = NULL;
 	struct rpc_pipe_msg msg;
 	struct bl_msg_hdr bl_msg = {
@@ -119,7 +119,7 @@ nfs4_blk_decode_device(struct nfs_server *server,
 	uint8_t *dataptr;
 	DECLARE_WAITQUEUE(wq, current);
 	struct bl_dev_msg *reply = &bl_mount_reply;
-	int offset, len, i;
+	int offset, len, i, rc;
 
 	dprintk("%s CREATING PIPEFS MESSAGE\n", __func__);
 	dprintk("%s: deviceid: %s, mincount: %d\n", __func__, dev->dev_id.data,
@@ -146,8 +146,10 @@ nfs4_blk_decode_device(struct nfs_server *server,
 
 	dprintk("%s CALLING USERSPACE DAEMON\n", __func__);
 	add_wait_queue(&bl_wq, &wq);
-	if (rpc_queue_upcall(bl_device_pipe->d_inode, &msg) < 0) {
+	rc = rpc_queue_upcall(bl_device_pipe->d_inode, &msg);
+	if (rc < 0) {
 		remove_wait_queue(&bl_wq, &wq);
+		rv = ERR_PTR(rc);
 		goto out;
 	}
 
@@ -165,8 +167,9 @@ nfs4_blk_decode_device(struct nfs_server *server,
 
 	bd = nfs4_blkdev_get(MKDEV(reply->major, reply->minor));
 	if (IS_ERR(bd)) {
-		dprintk("%s failed to open device : %ld\n",
-			__func__, PTR_ERR(bd));
+		rc = PTR_ERR(bd);
+		dprintk("%s failed to open device : %d\n", __func__, rc);
+		rv = ERR_PTR(rc);
 		goto out;
 	}
 
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] pnfsblock: fix return code confusion
  2011-09-09 16:33 [PATCH] pnfsblock: fix return code confusion Jim Rees
@ 2011-09-10  7:29 ` Benny Halevy
  0 siblings, 0 replies; 2+ messages in thread
From: Benny Halevy @ 2011-09-10  7:29 UTC (permalink / raw)
  To: Jim Rees; +Cc: linux-nfs, peter honeyman

On 2011-09-09 09:33, Jim Rees wrote:
> Always return PTR_ERR, not NULL, from nfs4_blk_get_deviceinfo and
> nfs4_blk_decode_device.
> 
> Check for IS_ERR, not NULL, in bl_set_layoutdriver when calling
> nfs4_blk_get_deviceinfo.
> 
> Signed-off-by: Jim Rees <rees@umich.edu>

merged. thanks!

Benny

> ---
>  fs/nfs/blocklayout/blocklayout.c    |   20 ++++++++++++--------
>  fs/nfs/blocklayout/blocklayoutdev.c |   13 ++++++++-----
>  2 files changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/nfs/blocklayout/blocklayout.c b/fs/nfs/blocklayout/blocklayout.c
> index 9143e61..7debb55 100644
> --- a/fs/nfs/blocklayout/blocklayout.c
> +++ b/fs/nfs/blocklayout/blocklayout.c
> @@ -794,7 +794,7 @@ nfs4_blk_get_deviceinfo(struct nfs_server *server, const struct nfs_fh *fh,
>  			struct nfs4_deviceid *d_id)
>  {
>  	struct pnfs_device *dev;
> -	struct pnfs_block_dev *rv = NULL;
> +	struct pnfs_block_dev *rv;
>  	u32 max_resp_sz;
>  	int max_pages;
>  	struct page **pages = NULL;
> @@ -812,18 +812,20 @@ nfs4_blk_get_deviceinfo(struct nfs_server *server, const struct nfs_fh *fh,
>  	dev = kmalloc(sizeof(*dev), GFP_NOFS);
>  	if (!dev) {
>  		dprintk("%s kmalloc failed\n", __func__);
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  	}
>  
>  	pages = kzalloc(max_pages * sizeof(struct page *), GFP_NOFS);
>  	if (pages == NULL) {
>  		kfree(dev);
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);
>  	}
>  	for (i = 0; i < max_pages; i++) {
>  		pages[i] = alloc_page(GFP_NOFS);
> -		if (!pages[i])
> +		if (!pages[i]) {
> +			rv = ERR_PTR(-ENOMEM);
>  			goto out_free;
> +		}
>  	}
>  
>  	memcpy(&dev->dev_id, d_id, sizeof(*d_id));
> @@ -836,8 +838,10 @@ nfs4_blk_get_deviceinfo(struct nfs_server *server, const struct nfs_fh *fh,
>  	dprintk("%s: dev_id: %s\n", __func__, dev->dev_id.data);
>  	rc = nfs4_proc_getdeviceinfo(server, dev);
>  	dprintk("%s getdevice info returns %d\n", __func__, rc);
> -	if (rc)
> +	if (rc) {
> +		rv = ERR_PTR(rc);
>  		goto out_free;
> +	}
>  
>  	rv = nfs4_blk_decode_device(server, dev);
>   out_free:
> @@ -855,7 +859,7 @@ bl_set_layoutdriver(struct nfs_server *server, const struct nfs_fh *fh)
>  	struct pnfs_devicelist *dlist = NULL;
>  	struct pnfs_block_dev *bdev;
>  	LIST_HEAD(block_disklist);
> -	int status = 0, i;
> +	int status, i;
>  
>  	dprintk("%s enter\n", __func__);
>  
> @@ -887,8 +891,8 @@ bl_set_layoutdriver(struct nfs_server *server, const struct nfs_fh *fh)
>  		for (i = 0; i < dlist->num_devs; i++) {
>  			bdev = nfs4_blk_get_deviceinfo(server, fh,
>  						       &dlist->dev_id[i]);
> -			if (!bdev) {
> -				status = -ENODEV;
> +			if (IS_ERR(bdev)) {
> +				status = PTR_ERR(bdev);
>  				goto out_error;
>  			}
>  			spin_lock(&b_mt_id->bm_lock);
> diff --git a/fs/nfs/blocklayout/blocklayoutdev.c b/fs/nfs/blocklayout/blocklayoutdev.c
> index 46a2ce6..d08ba91 100644
> --- a/fs/nfs/blocklayout/blocklayoutdev.c
> +++ b/fs/nfs/blocklayout/blocklayoutdev.c
> @@ -109,7 +109,7 @@ struct pnfs_block_dev *
>  nfs4_blk_decode_device(struct nfs_server *server,
>  		       struct pnfs_device *dev)
>  {
> -	struct pnfs_block_dev *rv = NULL;
> +	struct pnfs_block_dev *rv;
>  	struct block_device *bd = NULL;
>  	struct rpc_pipe_msg msg;
>  	struct bl_msg_hdr bl_msg = {
> @@ -119,7 +119,7 @@ nfs4_blk_decode_device(struct nfs_server *server,
>  	uint8_t *dataptr;
>  	DECLARE_WAITQUEUE(wq, current);
>  	struct bl_dev_msg *reply = &bl_mount_reply;
> -	int offset, len, i;
> +	int offset, len, i, rc;
>  
>  	dprintk("%s CREATING PIPEFS MESSAGE\n", __func__);
>  	dprintk("%s: deviceid: %s, mincount: %d\n", __func__, dev->dev_id.data,
> @@ -146,8 +146,10 @@ nfs4_blk_decode_device(struct nfs_server *server,
>  
>  	dprintk("%s CALLING USERSPACE DAEMON\n", __func__);
>  	add_wait_queue(&bl_wq, &wq);
> -	if (rpc_queue_upcall(bl_device_pipe->d_inode, &msg) < 0) {
> +	rc = rpc_queue_upcall(bl_device_pipe->d_inode, &msg);
> +	if (rc < 0) {
>  		remove_wait_queue(&bl_wq, &wq);
> +		rv = ERR_PTR(rc);
>  		goto out;
>  	}
>  
> @@ -165,8 +167,9 @@ nfs4_blk_decode_device(struct nfs_server *server,
>  
>  	bd = nfs4_blkdev_get(MKDEV(reply->major, reply->minor));
>  	if (IS_ERR(bd)) {
> -		dprintk("%s failed to open device : %ld\n",
> -			__func__, PTR_ERR(bd));
> +		rc = PTR_ERR(bd);
> +		dprintk("%s failed to open device : %d\n", __func__, rc);
> +		rv = ERR_PTR(rc);
>  		goto out;
>  	}
>  

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2011-09-10  7:29 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-09 16:33 [PATCH] pnfsblock: fix return code confusion Jim Rees
2011-09-10  7:29 ` Benny Halevy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).