[PATCH 2/2] Ext3: add necessary check in case IO error happens

[PATCH 2/2] Ext3: add necessary check in case IO error happens

[shilong wang]

From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>

As we know IO ERROR may happen when the function 'sb_getblk' is
called. Add necessary check for it.

The patch also fix a coding style problem.

Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
---
 fs/ext3/inode.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
index ff574b4..59b6178 100644
--- a/fs/ext3/inode.c
+++ b/fs/ext3/inode.c
@@ -676,6 +676,10 @@ static int ext3_alloc_branch(handle_t *handle,
struct inode *inode,
 		 * parent to disk.
 		 */
 		bh = sb_getblk(inode->i_sb, new_blocks[n-1]);
+		if (!bh) {
+			err = -EIO;
+			goto failed;
+		}
 		branch[n].bh = bh;
 		lock_buffer(bh);
 		BUFFER_TRACE(bh, "call get_create_access");
@@ -717,7 +721,7 @@ failed:
 		BUFFER_TRACE(branch[i].bh, "call journal_forget");
 		ext3_journal_forget(handle, branch[i].bh);
 	}
-	for (i = 0; i <indirect_blks; i++)
+	for (i = 0; i < indirect_blks; i++)
 		ext3_free_blocks(handle, inode, new_blocks[i], 1);

 	ext3_free_blocks(handle, inode, new_blocks[i], num);
-- 
1.7.7.6

Re: [PATCH 2/2] Ext3: add necessary check in case IO error happens

[Jan Kara]

On Fri 11-01-13 03:58:28, shilong wang wrote:
> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> 
> As we know IO ERROR may happen when the function 'sb_getblk' is
> called. Add necessary check for it.
> 
> The patch also fix a coding style problem.
  Thanks for the patch. I think returning ENOMEM instead of EIO would be
better. Otherwise the patch looks OK.

								Honza
> 
> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
> ---
>  fs/ext3/inode.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
> index ff574b4..59b6178 100644
> --- a/fs/ext3/inode.c
> +++ b/fs/ext3/inode.c
> @@ -676,6 +676,10 @@ static int ext3_alloc_branch(handle_t *handle,
> struct inode *inode,
>  		 * parent to disk.
>  		 */
>  		bh = sb_getblk(inode->i_sb, new_blocks[n-1]);
> +		if (!bh) {
> +			err = -EIO;
> +			goto failed;
> +		}
>  		branch[n].bh = bh;
>  		lock_buffer(bh);
>  		BUFFER_TRACE(bh, "call get_create_access");
> @@ -717,7 +721,7 @@ failed:
>  		BUFFER_TRACE(branch[i].bh, "call journal_forget");
>  		ext3_journal_forget(handle, branch[i].bh);
>  	}
> -	for (i = 0; i <indirect_blks; i++)
> +	for (i = 0; i < indirect_blks; i++)
>  		ext3_free_blocks(handle, inode, new_blocks[i], 1);
> 
>  	ext3_free_blocks(handle, inode, new_blocks[i], num);
> -- 
> 1.7.7.6
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 2/2] Ext3: add necessary check in case IO error happens

[shilong wang]

Thanks very much for your reply. I will resend the patch later,
Besides, i find there are many places where EIO is used when
'sb_getblk' fails. Shall i replace all of them with ENOMEM?
2013/1/11 Jan Kara <jack@suse.cz>:
> On Fri 11-01-13 03:58:28, shilong wang wrote:
>> From: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
>>
>> As we know IO ERROR may happen when the function 'sb_getblk' is
>> called. Add necessary check for it.
>>
>> The patch also fix a coding style problem.
>   Thanks for the patch. I think returning ENOMEM instead of EIO would be
> better. Otherwise the patch looks OK.
>
>                                                                 Honza
>>
>> Signed-off-by: Wang Shilong <wangsl-fnst@cn.fujitsu.com>
>> ---
>>  fs/ext3/inode.c |    6 +++++-
>>  1 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ext3/inode.c b/fs/ext3/inode.c
>> index ff574b4..59b6178 100644
>> --- a/fs/ext3/inode.c
>> +++ b/fs/ext3/inode.c
>> @@ -676,6 +676,10 @@ static int ext3_alloc_branch(handle_t *handle,
>> struct inode *inode,
>>                * parent to disk.
>>                */
>>               bh = sb_getblk(inode->i_sb, new_blocks[n-1]);
>> +             if (!bh) {
>> +                     err = -EIO;
>> +                     goto failed;
>> +             }
>>               branch[n].bh = bh;
>>               lock_buffer(bh);
>>               BUFFER_TRACE(bh, "call get_create_access");
>> @@ -717,7 +721,7 @@ failed:
>>               BUFFER_TRACE(branch[i].bh, "call journal_forget");
>>               ext3_journal_forget(handle, branch[i].bh);
>>       }
>> -     for (i = 0; i <indirect_blks; i++)
>> +     for (i = 0; i < indirect_blks; i++)
>>               ext3_free_blocks(handle, inode, new_blocks[i], 1);
>>
>>       ext3_free_blocks(handle, inode, new_blocks[i], num);
>> --
>> 1.7.7.6
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> Jan Kara <jack@suse.cz>
> SUSE Labs, CR

Re: [PATCH 2/2] Ext3: add necessary check in case IO error happens

[Theodore Ts'o]

On Fri, Jan 11, 2013 at 05:26:33PM +0100, Jan Kara wrote:
>   Thanks for the patch. I think returning ENOMEM instead of EIO would be
> better. Otherwise the patch looks OK.

This may be because fs/ext4/inode.c was returning EIO in similar
circumstances.  I agree ENOMEM is a better error code.  I'll fix up
ext4....

						- Ted