On Fri, Aug 21, 2009 at 04:06:59PM +0200, Jan Kara wrote: > > Hi, > > > > > I also have commented a possible bug in existing ext2 code, marked with XXX. > > Looks good, except: > > > > > +int ext2_setsize(struct inode *inode, loff_t newsize) > > This could be static. > > > > > @@ -1459,8 +1540,15 @@ int ext2_setattr(struct dentry *dentry, > > > if (error) > > > return error; > > > } > > > - error = inode_setattr(inode, iattr); > > > + if (iattr->ia_valid & ATTR_SIZE) { > > > + error = ext2_setsize(inode, iattr->ia_size); > > > + if (error) > > > + return error; > > > + } > > > + generic_setattr(inode, iattr); > > Here, we should store the error code I suppose... > Ah, I was confused. generic_setattr() returns void. But then remove > the check !error from: > if (!error && (iattr->ia_valid & ATTR_MODE)) > which just follows the generic_setattr(). That's what made me think > generic_setattr() returns something :) Yep, good suggestion. Andrew please add this incremental patch --- fs/ext2/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: linux-2.6/fs/ext2/inode.c =================================================================== --- linux-2.6.orig/fs/ext2/inode.c +++ linux-2.6/fs/ext2/inode.c @@ -1546,7 +1546,7 @@ int ext2_setattr(struct dentry *dentry, return error; } generic_setattr(inode, iattr); - if (!error && (iattr->ia_valid & ATTR_MODE)) + if (iattr->ia_valid & ATTR_MODE) error = ext2_acl_chmod(inode); mark_inode_dirty(inode); -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html