lkml.org 
[lkml]   [2000]   [Aug]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
Subject[PATCH] 2.2: Consistency in ext2 error setting
This is a patch from Andreas Dilger <adilger@turbolabs.com>.
It claims to fix some corner cases where an error code may not be
returned properly. Here's the description:

--------------------------------------------------------------------------
ext2_bread always sets err to some sane value. I traced this through:

ext2_bread() calls ext2_getblk to set *err or sets *err = -EIO;
ext2_getblk() calls ext2_get_block to set *err;
ext2_get_block() returns 0, -EIO, or {inode,block}_getblk _may_ set err;

The inode_getblk and block_getblk functions themselves do not always set *err,
although err = 0 before they are called from ext2_get_block(), so there is
_always_ a return value set from ext2_get_block().

The ext2_get_block() function may call block_getblk() several times without
checking the return value, so it _appears_ it would overwrite the last error
value. However, one (non-obvious) thing to note is that bh = NULL on error
return* from {inode,block}_getblk(), and calls to block_getblk() are a no-op
if bh == NULL.

(*) There was one (probably very rare) error case where block_getblk()
might return non-NULL: after bforget(result) jumps back to repeat: and
then we get an error with ext2_alloc_block() - we return a bad result.
I also fixed this in the attached patch.

I also found that ext2_alloc_block() did not set a return code in the
(#undef'd) PREALLOC path, and inode_getblk() set a return value instead
of using the error from ext2_alloc_block(). I also verified that
ext2_alloc_block() itself always sets an error code.
--------------------------------------------------------------------------

Index: fs/ext2/balloc.c
--- fs/ext2/balloc.c.prev
+++ fs/ext2/balloc.c Sat Jun 17 17:48:19 2000
@@ -479,9 +479,6 @@ repeat:
i = 0;
gdp = ext2_get_group_desc (sb, i, &bh2);
- if (!gdp) {
- *err = -EIO;
- unlock_super (sb);
- return 0;
- }
+ if (!gdp)
+ goto io_error;
if (le16_to_cpu(gdp->bg_free_blocks_count) > 0)
break;
Index: fs/ext2/inode.c
--- fs/ext2/inode.c.prev
+++ fs/ext2/inode.c Sat Jun 17 17:48:19 2000
@@ -132,4 +132,5 @@ static int ext2_alloc_block (struct inod
mark_buffer_dirty(bh, 1);
brelse (bh);
+ *err = 0;
} else {
ext2_discard_prealloc (inode);
@@ -207,4 +208,5 @@ int ext2_bmap (struct inode * inode, int
}

+/* must return NULL on error */
static struct buffer_head * inode_getblk (struct inode * inode, int nr,
int create, int new_block, int * err)
@@ -219,5 +221,5 @@ repeat:
tmp = *p;
if (tmp) {
- struct buffer_head * result = getblk (inode->i_dev, tmp, inode->i_sb->s_blocksize);
+ result = getblk (inode->i_dev, tmp, inode->i_sb->s_blocksize);
if (tmp == *p)
return result;
--
Chip Salzenberg - a.k.a. - <chip@valinux.com>
"I wanted to play hopscotch with the impenetrable mystery of existence,
but he stepped in a wormhole and had to go in early." // MST3K
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-03-22 12:38    [W:0.091 / U:0.708 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site