* [PATCH] ext4: deleted unnecessary assignments and useless "if" statement
@ 2013-06-20 5:44 jon ernst
2013-06-20 14:44 ` Theodore Ts'o
0 siblings, 1 reply; 6+ messages in thread
From: jon ernst @ 2013-06-20 5:44 UTC (permalink / raw)
To: linux-ext4
comparing unsigned variable with 0 always return false.
err = 0 is duplicated and unnecessary.
Signed-off-by: "Jon Ernst" <jonernst07@gmx.com>
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1098,8 +1098,6 @@ static int ext4_write_end(struct file *f
if (i_size_changed)
ext4_mark_inode_dirty(handle, inode);
- if (copied < 0)
- ret = copied;
if (pos + len > inode->i_size && ext4_can_truncate(inode))
/* if we have allocated more blocks and copied
* less. We will have blocks allocated outside
@@ -3365,7 +3363,6 @@ int ext4_block_zero_page_range(handle_t
pos += blocksize;
}
- err = 0;
if (buffer_freed(bh)) {
BUFFER_TRACE(bh, "freed: skip");
goto unlock;
@@ -3405,7 +3402,6 @@ int ext4_block_zero_page_range(handle_t
BUFFER_TRACE(bh, "zeroed end of block");
- err = 0;
if (ext4_should_journal_data(inode)) {
err = ext4_handle_dirty_metadata(handle, inode, bh);
} else {
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ext4: deleted unnecessary assignments and useless "if" statement 2013-06-20 5:44 [PATCH] ext4: deleted unnecessary assignments and useless "if" statement jon ernst @ 2013-06-20 14:44 ` Theodore Ts'o 2013-06-20 14:52 ` [PATCH] ext4: check error return from ext4_write_inline_data_end() Theodore Ts'o 0 siblings, 1 reply; 6+ messages in thread From: Theodore Ts'o @ 2013-06-20 14:44 UTC (permalink / raw) To: jon ernst; +Cc: linux-ext4 On Thu, Jun 20, 2013 at 01:44:56AM -0400, jon ernst wrote: > comparing unsigned variable with 0 always return false. > err = 0 is duplicated and unnecessary. > > Signed-off-by: "Jon Ernst" <jonernst07@gmx.com> I've applied this with a few changes. (a) your patch was white space corrupted (b) one of the err = 0 which you removed was actually necessary. I've reworked the error handling in ext4_block_zero_page_range() so it's a bit more obvious. Also your change showed up a potential issue with ext4_write_end(), in that ext4_write_inline_data_end() returns an int, which can be negative in the case of an error, and it's assigning this to copied, which is an unsigned int. - Ted >From 6e4837a3e8bc04a1c49f8668c32ce3239968a8ce Mon Sep 17 00:00:00 2001 From: jon ernst <jonernst07@gmx.com> Date: Thu, 20 Jun 2013 10:42:05 -0400 Subject: [PATCH] ext4: delete unnecessary C statements Comparing unsigned variable with 0 always returns false. err = 0 is duplicated and unnecessary. Signed-off-by: "Jon Ernst" <jonernst07@gmx.com> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> ---- --- fs/ext4/inode.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 78cf398..baf5c2b 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1100,8 +1100,6 @@ static int ext4_write_end(struct file *file, if (i_size_changed) ext4_mark_inode_dirty(handle, inode); - if (copied < 0) - ret = copied; if (pos + len > inode->i_size && ext4_can_truncate(inode)) /* if we have allocated more blocks and copied * less. We will have blocks allocated outside @@ -3336,7 +3334,7 @@ int ext4_block_zero_page_range(handle_t *handle, struct inode *inode = mapping->host; struct buffer_head *bh; struct page *page; - int err = 0; + int err; page = find_or_create_page(mapping, from >> PAGE_CACHE_SHIFT, mapping_gfp_mask(mapping) & ~__GFP_FS); @@ -3366,13 +3364,10 @@ int ext4_block_zero_page_range(handle_t *handle, iblock++; pos += blocksize; } - - err = 0; if (buffer_freed(bh)) { BUFFER_TRACE(bh, "freed: skip"); goto unlock; } - if (!buffer_mapped(bh)) { BUFFER_TRACE(bh, "unmapped"); ext4_get_block(inode, iblock, bh, 0); @@ -3395,22 +3390,19 @@ int ext4_block_zero_page_range(handle_t *handle, if (!buffer_uptodate(bh)) goto unlock; } - if (ext4_should_journal_data(inode)) { BUFFER_TRACE(bh, "get write access"); err = ext4_journal_get_write_access(handle, bh); if (err) goto unlock; } - zero_user(page, offset, length); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] ext4: check error return from ext4_write_inline_data_end() 2013-06-20 14:44 ` Theodore Ts'o @ 2013-06-20 14:52 ` Theodore Ts'o 2013-06-20 19:45 ` [PATCH -v3] ext4: delete unnecessary C statements Theodore Ts'o 2013-06-24 13:40 ` [PATCH] ext4: check error return from ext4_write_inline_data_end() Zheng Liu 0 siblings, 2 replies; 6+ messages in thread From: Theodore Ts'o @ 2013-06-20 14:52 UTC (permalink / raw) To: Ext4 Developers List; +Cc: jonernst07, Theodore Ts'o, Zheng Liu, stable The function ext4_write_inline_data_end() can return an error. So we need to assign it to a signed integer variable to check for an error return (since copied is an unsigned int). Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> Cc: Zheng Liu <wenqing.lz@taobao.com> Cc: stable@vger.kernel.org --- fs/ext4/inode.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index baf5c2b..567a734 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1061,10 +1061,13 @@ static int ext4_write_end(struct file *file, } } - if (ext4_has_inline_data(inode)) - copied = ext4_write_inline_data_end(inode, pos, len, - copied, page); - else + if (ext4_has_inline_data(inode)) { + ret = ext4_write_inline_data_end(inode, pos, len, + copied, page); + if (ret < 0) + goto errout; + copied = ret; + } else copied = block_write_end(file, mapping, pos, len, copied, page, fsdata); -- 1.7.12.rc0.22.gcdd159b ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH -v3] ext4: delete unnecessary C statements 2013-06-20 14:52 ` [PATCH] ext4: check error return from ext4_write_inline_data_end() Theodore Ts'o @ 2013-06-20 19:45 ` Theodore Ts'o 2013-06-24 13:52 ` Zheng Liu 2013-06-24 13:40 ` [PATCH] ext4: check error return from ext4_write_inline_data_end() Zheng Liu 1 sibling, 1 reply; 6+ messages in thread From: Theodore Ts'o @ 2013-06-20 19:45 UTC (permalink / raw) To: Ext4 Developers List; +Cc: jon ernst, Theodore Ts'o From: jon ernst <jonernst07@gmx.com> Comparing unsigned variable with 0 always returns false. err = 0 is duplicated and unnecessary. [ tytso: Also cleaned up error handling in ext4_block_zero_page_range() ] Signed-off-by: "Jon Ernst" <jonernst07@gmx.com> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> --- fs/ext4/inode.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 78cf398..67963cf 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1100,8 +1100,6 @@ static int ext4_write_end(struct file *file, if (i_size_changed) ext4_mark_inode_dirty(handle, inode); - if (copied < 0) - ret = copied; if (pos + len > inode->i_size && ext4_can_truncate(inode)) /* if we have allocated more blocks and copied * less. We will have blocks allocated outside @@ -3366,13 +3364,10 @@ int ext4_block_zero_page_range(handle_t *handle, iblock++; pos += blocksize; } - - err = 0; if (buffer_freed(bh)) { BUFFER_TRACE(bh, "freed: skip"); goto unlock; } - if (!buffer_mapped(bh)) { BUFFER_TRACE(bh, "unmapped"); ext4_get_block(inode, iblock, bh, 0); @@ -3395,22 +3390,19 @@ int ext4_block_zero_page_range(handle_t *handle, if (!buffer_uptodate(bh)) goto unlock; } - if (ext4_should_journal_data(inode)) { BUFFER_TRACE(bh, "get write access"); err = ext4_journal_get_write_access(handle, bh); if (err) goto unlock; } - zero_user(page, offset, length); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH -v3] ext4: delete unnecessary C statements 2013-06-20 19:45 ` [PATCH -v3] ext4: delete unnecessary C statements Theodore Ts'o @ 2013-06-24 13:52 ` Zheng Liu 0 siblings, 0 replies; 6+ messages in thread From: Zheng Liu @ 2013-06-24 13:52 UTC (permalink / raw) To: Theodore Ts'o; +Cc: Ext4 Developers List, jon ernst On Thu, Jun 20, 2013 at 03:45:48PM -0400, Theodore Ts'o wrote: > From: jon ernst <jonernst07@gmx.com> > > Comparing unsigned variable with 0 always returns false. > err = 0 is duplicated and unnecessary. > > [ tytso: Also cleaned up error handling in ext4_block_zero_page_range() ] > > Signed-off-by: "Jon Ernst" <jonernst07@gmx.com> > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> Reviewed-by: Zheng Liu <wenqing.lz@taobao.com> - Zheng > --- > fs/ext4/inode.c | 10 +--------- > 1 file changed, 1 insertion(+), 9 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 78cf398..67963cf 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -1100,8 +1100,6 @@ static int ext4_write_end(struct file *file, > if (i_size_changed) > ext4_mark_inode_dirty(handle, inode); > > - if (copied < 0) > - ret = copied; > if (pos + len > inode->i_size && ext4_can_truncate(inode)) > /* if we have allocated more blocks and copied > * less. We will have blocks allocated outside > @@ -3366,13 +3364,10 @@ int ext4_block_zero_page_range(handle_t *handle, > iblock++; > pos += blocksize; > } > - > - err = 0; > if (buffer_freed(bh)) { > BUFFER_TRACE(bh, "freed: skip"); > goto unlock; > } > - > if (!buffer_mapped(bh)) { > BUFFER_TRACE(bh, "unmapped"); > ext4_get_block(inode, iblock, bh, 0); > @@ -3395,22 +3390,19 @@ int ext4_block_zero_page_range(handle_t *handle, > if (!buffer_uptodate(bh)) > goto unlock; > } > - > if (ext4_should_journal_data(inode)) { > BUFFER_TRACE(bh, "get write access"); > err = ext4_journal_get_write_access(handle, bh); > if (err) > goto unlock; > } > - > zero_user(page, offset, length); > - > BUFFER_TRACE(bh, "zeroed end of block"); > > - err = 0; > if (ext4_should_journal_data(inode)) { > err = ext4_handle_dirty_metadata(handle, inode, bh); > } else { > + err = 0; > mark_buffer_dirty(bh); > if (ext4_test_inode_state(inode, EXT4_STATE_ORDERED_MODE)) > err = ext4_jbd2_file_inode(handle, inode); > -- > 1.7.12.rc0.22.gcdd159b > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ext4: check error return from ext4_write_inline_data_end() 2013-06-20 14:52 ` [PATCH] ext4: check error return from ext4_write_inline_data_end() Theodore Ts'o 2013-06-20 19:45 ` [PATCH -v3] ext4: delete unnecessary C statements Theodore Ts'o @ 2013-06-24 13:40 ` Zheng Liu 1 sibling, 0 replies; 6+ messages in thread From: Zheng Liu @ 2013-06-24 13:40 UTC (permalink / raw) To: Theodore Ts'o; +Cc: Ext4 Developers List, jonernst07, Zheng Liu, stable On Thu, Jun 20, 2013 at 10:52:23AM -0400, Theodore Ts'o wrote: > The function ext4_write_inline_data_end() can return an error. So we > need to assign it to a signed integer variable to check for an error > return (since copied is an unsigned int). > > Signed-off-by: "Theodore Ts'o" <tytso@mit.edu> > Cc: Zheng Liu <wenqing.lz@taobao.com> > Cc: stable@vger.kernel.org Sorry for the late reply. Thanks for fixing this. The patch looks good to me. Reviewed-by: Zheng Liu <wenqing.lz@taobao.com> - Zheng > --- > fs/ext4/inode.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index baf5c2b..567a734 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -1061,10 +1061,13 @@ static int ext4_write_end(struct file *file, > } > } > > - if (ext4_has_inline_data(inode)) > - copied = ext4_write_inline_data_end(inode, pos, len, > - copied, page); > - else > + if (ext4_has_inline_data(inode)) { > + ret = ext4_write_inline_data_end(inode, pos, len, > + copied, page); > + if (ret < 0) > + goto errout; > + copied = ret; > + } else > copied = block_write_end(file, mapping, pos, > len, copied, page, fsdata); > > -- > 1.7.12.rc0.22.gcdd159b > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-24 13:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-20 5:44 [PATCH] ext4: deleted unnecessary assignments and useless "if" statement jon ernst 2013-06-20 14:44 ` Theodore Ts'o 2013-06-20 14:52 ` [PATCH] ext4: check error return from ext4_write_inline_data_end() Theodore Ts'o 2013-06-20 19:45 ` [PATCH -v3] ext4: delete unnecessary C statements Theodore Ts'o 2013-06-24 13:52 ` Zheng Liu 2013-06-24 13:40 ` [PATCH] ext4: check error return from ext4_write_inline_data_end() Zheng Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).