[PATCH 3.17-rc2] fs: namespace: suppress 'may be used uninitialized' warnings

[PATCH 3.17-rc2] fs: namespace: suppress 'may be used uninitialized' warnings

[rtg.canonical]

From: Tim Gardner <tim.gardner@canonical.com>

The gcc version 4.9.1 compiler complains Even though it isn't possible for
these variables to not get initialized before they are used.

fs/namespace.c: In function ‘SyS_mount’:
fs/namespace.c:2720:8: warning: ‘kernel_dev’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  ret = do_mount(kernel_dev, kernel_dir->name, kernel_type, flags,
        ^
fs/namespace.c:2699:8: note: ‘kernel_dev’ was declared here
  char *kernel_dev;
        ^
fs/namespace.c:2720:8: warning: ‘kernel_type’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  ret = do_mount(kernel_dev, kernel_dir->name, kernel_type, flags,
        ^
fs/namespace.c:2697:8: note: ‘kernel_type’ was declared here
  char *kernel_type;
        ^
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
 fs/namespace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 20232e4..8681260 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2694,9 +2694,9 @@ SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
 		char __user *, type, unsigned long, flags, void __user *, data)
 {
 	int ret;
-	char *kernel_type;
+	char *uninitialized_var(kernel_type);
 	struct filename *kernel_dir;
-	char *kernel_dev;
+	char *uninitialized_var(kernel_dev);
 	unsigned long data_page;
 
 	ret = copy_mount_string(type, &kernel_type);
-- 
2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3.17-rc2] fs: namespace: suppress 'may be used uninitialized' warnings

[Al Viro]

On Wed, Aug 27, 2014 at 01:51:05PM -0600, rtg.canonical@gmail.com wrote:
> From: Tim Gardner <tim.gardner@canonical.com>
> 
> The gcc version 4.9.1 compiler complains Even though it isn't possible for
> these variables to not get initialized before they are used.

Blanket NAK, at that - any patches adding that 'uninitialized_...'
garbage in fs/*.c will be rejected and, should they slip in in some
other way - reverted.  Consider that as a local policy.

[PATCH v2] fs: namespace: suppress 'may be used uninitialized' warnings

[rtg.canonical]

From: Tim Gardner <tim.gardner@canonical.com>

The gcc version 4.9.1 compiler complains even though it isn't possible for
these variables to not get initialized before they are used.

fs/namespace.c: In function ‘SyS_mount’:
fs/namespace.c:2720:8: warning: ‘kernel_dev’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  ret = do_mount(kernel_dev, kernel_dir->name, kernel_type, flags,
        ^
fs/namespace.c:2699:8: note: ‘kernel_dev’ was declared here
  char *kernel_dev;
        ^
fs/namespace.c:2720:8: warning: ‘kernel_type’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  ret = do_mount(kernel_dev, kernel_dir->name, kernel_type, flags,
        ^
fs/namespace.c:2697:8: note: ‘kernel_type’ was declared here
  char *kernel_type;
        ^
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---

V1 - use of the uninitialized_var() macro rejected.
V2 - assign automatic variables an initial value.

 fs/namespace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index a01c773..365a06d 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2665,9 +2665,9 @@ SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
 		char __user *, type, unsigned long, flags, void __user *, data)
 {
 	int ret;
-	char *kernel_type;
+	char *kernel_type = NULL;
 	struct filename *kernel_dir;
-	char *kernel_dev;
+	char *kernel_dev = NULL;
 	unsigned long data_page;
 
 	ret = copy_mount_string(type, &kernel_type);
-- 
2.1.0

Re: [PATCH v2] fs: namespace: suppress 'may be used uninitialized' warnings

[Al Viro]

On Thu, Aug 28, 2014 at 07:55:49AM -0600, rtg.canonical@gmail.com wrote:
> From: Tim Gardner <tim.gardner@canonical.com>
> 
> The gcc version 4.9.1 compiler complains even though it isn't possible for
> these variables to not get initialized before they are used.

Sigh...  The root cause of that shite is that copy_mount_string() is too
convoluted for gcc (piss-poor) detection of uninitialized variables.  And
yes, it is somewhat overcomplicated - it returns 0 or -E... *and* in former
case it returns NULL or a string as well, via a char ** argument.

The usual convention for such suckers is "return a pointer, using
ERR_PTR(-E...) to indicate an error".  We have all of 4 (four) callers,
all in fs/*.c (and nobody else could see that function, unless they manually
included fs/internal.h).

So let's turn that into
char *copy_mount_string(const void __user *data)
{
	return data ? strndup_user(data, PAGE_SIZE) : NULL;
}

and uses of that thing into
        kernel_type = copy_mount_string(type);
	ret = PTR_ERR(kernel_type);
        if (IS_ERR(kernel_type))
                goto out_type;
etc.

[PATCH v3] fs: namespace: suppress 'may be used uninitialized' warnings

[rtg.canonical]

From: Tim Gardner <tim.gardner@canonical.com>

The gcc version 4.9.1 compiler complains Even though it isn't possible for
these variables to not get initialized before they are used.

fs/namespace.c: In function ‘SyS_mount’:
fs/namespace.c:2720:8: warning: ‘kernel_dev’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  ret = do_mount(kernel_dev, kernel_dir->name, kernel_type, flags,
        ^
fs/namespace.c:2699:8: note: ‘kernel_dev’ was declared here
  char *kernel_dev;
        ^
fs/namespace.c:2720:8: warning: ‘kernel_type’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  ret = do_mount(kernel_dev, kernel_dir->name, kernel_type, flags,
        ^
fs/namespace.c:2697:8: note: ‘kernel_type’ was declared here
  char *kernel_type;
        ^

Fix the warnings by simplifying copy_mount_string() as suggested by Al Viro.

Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---

V1 - use of the uninitialized_var() macro rejected.
V2 - assign automatic variables an initial value.
V3 - minor surgery to simplify copy_mount_string(), thereby removing all doubt as to whether
     these variables are uninitialized.

 fs/compat.c    | 10 ++++++----
 fs/internal.h  |  2 +-
 fs/namespace.c | 26 ++++++++------------------
 3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/fs/compat.c b/fs/compat.c
index 66d3d3c..6205c24 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -797,8 +797,9 @@ COMPAT_SYSCALL_DEFINE5(mount, const char __user *, dev_name,
 	struct filename *dir;
 	int retval;
 
-	retval = copy_mount_string(type, &kernel_type);
-	if (retval < 0)
+	kernel_type = copy_mount_string(type);
+	retval = PTR_ERR(kernel_type);
+	if (IS_ERR(kernel_type))
 		goto out;
 
 	dir = getname(dir_name);
@@ -806,8 +807,9 @@ COMPAT_SYSCALL_DEFINE5(mount, const char __user *, dev_name,
 	if (IS_ERR(dir))
 		goto out1;
 
-	retval = copy_mount_string(dev_name, &kernel_dev);
-	if (retval < 0)
+	kernel_dev = copy_mount_string(dev_name);
+	retval = PTR_ERR(kernel_dev);
+	if (IS_ERR(kernel_dev))
 		goto out2;
 
 	retval = copy_mount_options(data, &data_page);
diff --git a/fs/internal.h b/fs/internal.h
index e325b4f..bd4ac19 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -51,7 +51,7 @@ extern int vfs_path_lookup(struct dentry *, struct vfsmount *,
  * namespace.c
  */
 extern int copy_mount_options(const void __user *, unsigned long *);
-extern int copy_mount_string(const void __user *, char **);
+extern char *copy_mount_string(const void __user *);
 
 extern struct vfsmount *lookup_mnt(struct path *);
 extern int finish_automount(struct vfsmount *, struct path *);
diff --git a/fs/namespace.c b/fs/namespace.c
index a01c773..152c972 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2390,21 +2390,9 @@ int copy_mount_options(const void __user * data, unsigned long *where)
 	return 0;
 }
 
-int copy_mount_string(const void __user *data, char **where)
+char *copy_mount_string(const void __user *data)
 {
-	char *tmp;
-
-	if (!data) {
-		*where = NULL;
-		return 0;
-	}
-
-	tmp = strndup_user(data, PAGE_SIZE);
-	if (IS_ERR(tmp))
-		return PTR_ERR(tmp);
-
-	*where = tmp;
-	return 0;
+	return data ? strndup_user(data, PAGE_SIZE) : NULL;
 }
 
 /*
@@ -2670,8 +2658,9 @@ SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
 	char *kernel_dev;
 	unsigned long data_page;
 
-	ret = copy_mount_string(type, &kernel_type);
-	if (ret < 0)
+	kernel_type = copy_mount_string(type);
+	ret = PTR_ERR(kernel_type);
+	if (IS_ERR(kernel_type))
 		goto out_type;
 
 	kernel_dir = getname(dir_name);
@@ -2680,8 +2669,9 @@ SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
 		goto out_dir;
 	}
 
-	ret = copy_mount_string(dev_name, &kernel_dev);
-	if (ret < 0)
+	kernel_dev = copy_mount_string(dev_name);
+	ret = PTR_ERR(kernel_dev);
+	if (IS_ERR(kernel_dev))
 		goto out_dev;
 
 	ret = copy_mount_options(data, &data_page);
-- 
2.1.0