| Message ID | 20121205054125.GC18885@thunk.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Wed, Dec 05, 2012 at 12:41:25AM -0500, Theodore Ts'o wrote: > On Tue, Dec 04, 2012 at 04:56:02PM +0300, Dan Carpenter wrote: > > It looks good. > > > > Like I mentioned before Smatch doesn't understand > > ext4_has_inline_data() so it still complains later on in the > > function. But it's now obvious to a human reader that there won't > > be a NULL dereference. > > This is what I plan to fold into the patch. It should make it easier > for gcc to produce optimized code, as well as being easier to > understand. Hopefully this should also keep smatch happy. > Yeah, moving the check for (inode_bh == NULL) forward is nice. Thanks. regards, dan carpenter -- 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
On Wed, Dec 05, 2012 at 11:31:41AM +0300, Dan Carpenter wrote: > > Yeah, moving the check for (inode_bh == NULL) forward is nice. > Thanks. Not that I really care about optimizing the error path that much, but getting a starting a handle only to immediately stop it again if inode_bh == NULL was just silly. :-) - Ted -- 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
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 52f715e..ae253a2 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1917,19 +1917,24 @@ static int __ext4_journalled_writepage(struct page *page, struct inode *inode = mapping->host; struct buffer_head *page_bufs = NULL; handle_t *handle = NULL; - int ret = 0; - int err; + int ret = 0, err = 0; + int inline_data = ext4_has_inline_data(inode); struct buffer_head *inode_bh = NULL; ClearPageChecked(page); - if (ext4_has_inline_data(inode)) { + if (inline_data) { BUG_ON(page->index != 0); BUG_ON(len > ext4_get_max_inline_size(inode)); inode_bh = ext4_journalled_write_inline_data(inode, len, page); + if (inode_bh == NULL) + goto out; } else { page_bufs = page_buffers(page); - BUG_ON(!page_bufs); + if (!page_bufs) { + BUG(); + goto out; + } walk_page_buffers(handle, page_bufs, 0, len, NULL, bget_one); } /* As soon as we unlock the page, it can go away, but we have @@ -1944,7 +1949,7 @@ static int __ext4_journalled_writepage(struct page *page, BUG_ON(!ext4_handle_valid(handle)); - if (ext4_has_inline_data(inode) && inode_bh) { + if (inline_data) { ret = ext4_journal_get_write_access(handle, inode_bh); err = ext4_handle_dirty_metadata(handle, inode, inode_bh);