From: Arnd Bergmann <arnd@arndb.de>
To: Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <yuchao0@huawei.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
        Fan Li <fanofcode.li@samsung.com>, Weichao Guo <guoweichao@huawei.com>,
        linux-f2fs-devel@lists.sourceforge.net
Subject: [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON
Date: Tue, 18 Oct 2016 00:05:33 +0200
Message-Id: <20161017220557.1688282-4-arnd@arndb.de>
In-Reply-To: <20161017220342.1627073-1-arnd@arndb.de>
References: <20161017220342.1627073-1-arnd@arndb.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2096
Lines: 56

gcc is unsure about the use of last_ofs_in_node, which might happen
without a prior initialization:

fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’:
fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {

I'm not sure about it either, so to shut up the warning I initialize
it to a known invalid -1u and later check for this, so we get a
runtime warning if we ever hit the uninitialized case.

It would be much better to reorganize the code in some form that
made it obvious to both the compiler and the reader that this
variable use it ok.

Since I only see the warning with gcc-4.9 but not any later version,
it's possible that the compiler is actually smarter than I am here
and has learned to see the code as correct, in which case this
patch could just be disregarded. It would certainly be helpful
to get an opinion from the maintainers on the matter.

Fixes: 46008c6d4232 ("f2fs: support in batch multi blocks preallocation")
Cc: Chao Yu <yuchao0@huawei.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/f2fs/data.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9ae194f..1b17de2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -696,6 +696,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		goto out;
 	}
 
+	/*
+	 * FIXME: without this, we get "warning: ‘last_ofs_in_node’ may be
+	 * used uninitialized". It's not clear whether that can actually
+	 * happen, so there is now a WARN_ON() checking for this.
+	 */
+	last_ofs_in_node = -1u;
 next_dnode:
 	if (create)
 		f2fs_lock_op(sbi);
@@ -796,6 +802,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map,
 		allocated = dn.node_changed;
 
 		map->m_len += dn.ofs_in_node - ofs_in_node;
+		WARN_ON(last_ofs_in_node == -1u);
 		if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) {
 			err = -ENOSPC;
 			goto sync_out;
-- 
2.9.0