[PATCH] cachefiles: fix __cachefiles_prepare_write() error handling

[PATCH] cachefiles: fix __cachefiles_prepare_write() error handling

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

clang points out that __cachefiles_prepare_write() returns an
uninitialized error code in one of the code paths:

fs/cachefiles/io.c:489:6: error: variable 'ret' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
        if (pos == 0)
            ^~~~~~~~
fs/cachefiles/io.c:492:6: note: uninitialized use occurs here
        if (ret < 0) {
            ^~~
fs/cachefiles/io.c:489:2: note: remove the 'if' if its condition is always true
        if (pos == 0)
        ^~~~~~~~~~~~~
fs/cachefiles/io.c:440:9: note: initialize the variable 'ret' to silence this warning
        int ret;
               ^

Rework to return zero for success here and skip the rest of the
function.

Fixes: 0443b01eccbb ("cachefiles: Implement the I/O routines")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/cachefiles/io.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
index 74ef4d1fc562..0fab313a604d 100644
--- a/fs/cachefiles/io.c
+++ b/fs/cachefiles/io.c
@@ -486,9 +486,11 @@ static int __cachefiles_prepare_write(struct netfs_cache_resources *cres,
 	/* Partially allocated, but insufficient space: cull. */
 	fscache_count_no_write_space();
 	pos = cachefiles_inject_remove_error();
-	if (pos == 0)
-		ret = vfs_fallocate(file, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-				    *_start, *_len);
+	if (pos != 0)
+		return 0;
+
+	ret = vfs_fallocate(file, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+			    *_start, *_len);
 	if (ret < 0) {
 		trace_cachefiles_io_error(object, file_inode(file), ret,
 					  cachefiles_trace_fallocate_error);
-- 
2.29.2

Re: [PATCH] cachefiles: fix __cachefiles_prepare_write() error handling

[David Howells]

Arnd Bergmann <arnd@kernel.org> wrote:

>  	pos = cachefiles_inject_remove_error();
> -	if (pos == 0)
> -		ret = vfs_fallocate(file, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> -				    *_start, *_len);
> +	if (pos != 0)
> +		return 0;
> +
> +	ret = vfs_fallocate(file, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> +			    *_start, *_len);
>  	if (ret < 0) {

The right fix is to use "ret" rather than "pos".  I've fixed this already on
my fscache-rewrite branch.  Hopefully I'll be able to push that over to my
next branch shortly.

David