Messages in this thread Patch in this message | | | Date | Tue, 18 Dec 2007 22:14:14 +0000 (GMT) | From | Hugh Dickins <> | Subject | [PATCH 3/4] unionfs: restructure unionfs_setattr |
| |
In order to fix unionfs truncation, we need to move the lower notify_change out of the loop in unionfs_setattr. But when I came to do that, I couldn't understand that loop at all: its continues and breaks and gotos are obscure, most particularly the "if (copyup || (bindex != bstart)) continue;" which seems to make a nonsense of the whole thing.
Here's my attempt to restructure it, prior to making the functional change in the next patch; but I may have misunderstood badly, please check - I've not tested it beyond my degenerate dirs=/tmpfs case, which hardly tests it.
Signed-off-by: Hugh Dickins <hugh@veritas.com> ---
fs/unionfs/inode.c | 64 ++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 36 deletions(-)
--- 2.6.24-rc5-mm1/fs/unionfs/inode.c 2007-12-05 10:38:38.000000000 +0000 +++ unionfs3/fs/unionfs/inode.c 2007-12-05 18:50:52.000000000 +0000 @@ -1004,11 +1004,10 @@ static int unionfs_setattr(struct dentry { int err = 0; struct dentry *lower_dentry; - struct inode *inode = NULL; - struct inode *lower_inode = NULL; + struct inode *inode; + struct inode *lower_inode; int bstart, bend, bindex; - int i; - int copyup = 0; + loff_t size; unionfs_read_lock(dentry->d_sb); unionfs_lock_dentry(dentry); @@ -1029,50 +1028,43 @@ static int unionfs_setattr(struct dentry if (ia->ia_valid & (ATTR_KILL_SUID | ATTR_KILL_SGID)) ia->ia_valid &= ~ATTR_MODE; - for (bindex = bstart; (bindex <= bend) || (bindex == bstart); - bindex++) { - lower_dentry = unionfs_lower_dentry_idx(dentry, bindex); - if (!lower_dentry) - continue; - BUG_ON(lower_dentry->d_inode == NULL); - + lower_dentry = unionfs_lower_dentry(dentry); + if (lower_dentry) { /* If the file is on a read only branch */ - if (is_robranch_super(dentry->d_sb, bindex) + if (is_robranch_super(dentry->d_sb, bstart) || IS_RDONLY(lower_dentry->d_inode)) { - if (copyup || (bindex != bstart)) - continue; - /* Only if its the leftmost file, copyup the file */ - for (i = bstart - 1; i >= 0; i--) { - loff_t size = i_size_read(dentry->d_inode); - if (ia->ia_valid & ATTR_SIZE) - size = ia->ia_size; + + if (ia->ia_valid & ATTR_SIZE) + size = ia->ia_size; + else + size = i_size_read(inode); + + for (bindex = bstart - 1; bindex >= 0; bindex--) { err = copyup_dentry(dentry->d_parent->d_inode, - dentry, bstart, i, + dentry, bstart, bindex, dentry->d_name.name, dentry->d_name.len, NULL, size); - - if (!err) { - copyup = 1; - lower_dentry = - unionfs_lower_dentry(dentry); + if (!err) break; - } - /* - * if error is in the leftmost branch, pass - * it up. - */ - if (i == 0) - goto out; } + if (err) + goto out; + lower_dentry = unionfs_lower_dentry(dentry); + } + } else { + for (bindex = bstart + 1; bindex <= bend; bindex++) { + lower_dentry = unionfs_lower_dentry_idx(dentry, bindex); + if (lower_dentry) + break; } - err = notify_change(lower_dentry, ia); - if (err) - goto out; - break; } + err = notify_change(lower_dentry, ia); + if (err) + goto out; + /* for mmap */ if (ia->ia_valid & ATTR_SIZE) { if (ia->ia_size != i_size_read(inode)) {
| |