Messages in this thread Patches in this message |  | | Date | Tue, 29 Aug 2000 18:16:55 -0700 | From | Chip Salzenberg <> | 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/
|  |