[PATCH 1/5] um: signal: move pid variable where needed

[PATCH 1/5] um: signal: move pid variable where needed

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

We have W=1 warnings on 64-bit because the pid is only
used in branches on 32-bit; move it inside to get rid of
the warnings.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/x86/um/signal.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/x86/um/signal.c b/arch/x86/um/signal.c
index 263e1d08f216..16ff097e790d 100644
--- a/arch/x86/um/signal.c
+++ b/arch/x86/um/signal.c
@@ -155,7 +155,7 @@ static int copy_sc_from_user(struct pt_regs *regs,
 			     struct sigcontext __user *from)
 {
 	struct sigcontext sc;
-	int err, pid;
+	int err;
 
 	/* Always make any pending restarted system calls return -EINTR */
 	current->restart_block.fn = do_no_restart_syscall;
@@ -201,10 +201,10 @@ static int copy_sc_from_user(struct pt_regs *regs,
 
 #undef GETREG
 
-	pid = userspace_pid[current_thread_info()->cpu];
 #ifdef CONFIG_X86_32
 	if (have_fpx_regs) {
 		struct user_fxsr_struct fpx;
+		int pid = userspace_pid[current_thread_info()->cpu];
 
 		err = copy_from_user(&fpx,
 			&((struct _fpstate __user *)sc.fpstate)->_fxsr_env[0],
@@ -240,7 +240,7 @@ static int copy_sc_to_user(struct sigcontext __user *to,
 {
 	struct sigcontext sc;
 	struct faultinfo * fi = &current->thread.arch.faultinfo;
-	int err, pid;
+	int err;
 	memset(&sc, 0, sizeof(struct sigcontext));
 
 #define PUTREG(regno, regname) sc.regname = regs->regs.gp[HOST_##regno]
@@ -288,10 +288,9 @@ static int copy_sc_to_user(struct sigcontext __user *to,
 	if (err)
 		return 1;
 
-	pid = userspace_pid[current_thread_info()->cpu];
-
 #ifdef CONFIG_X86_32
 	if (have_fpx_regs) {
+		int pid = userspace_pid[current_thread_info()->cpu];
 		struct user_fxsr_struct fpx;
 
 		err = save_fpx_registers(pid, (unsigned long *) &fpx);
-- 
2.44.0

[PATCH 2/5] um: slirp: remove set but unused variable 'pid'

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The code doesn't use 'pid' here, just remove it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/drivers/slirp_user.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/um/drivers/slirp_user.c b/arch/um/drivers/slirp_user.c
index 8f633e2e5f3d..97228aa080cb 100644
--- a/arch/um/drivers/slirp_user.c
+++ b/arch/um/drivers/slirp_user.c
@@ -49,7 +49,7 @@ static int slirp_tramp(char **argv, int fd)
 static int slirp_open(void *data)
 {
 	struct slirp_data *pri = data;
-	int fds[2], pid, err;
+	int fds[2], err;
 
 	err = os_pipe(fds, 1, 1);
 	if (err)
@@ -60,7 +60,6 @@ static int slirp_open(void *data)
 		printk(UM_KERN_ERR "slirp_tramp failed - errno = %d\n", -err);
 		goto out;
 	}
-	pid = err;
 
 	pri->slave = fds[1];
 	pri->slip.pos = 0;
-- 
2.44.0

[PATCH 3/5] um: vector: fix bpfflash parameter evaluation

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

With W=1 the build complains about a pointer compared to
zero, clearly the result should've been compared.

Fixes: 9807019a62dc ("um: Loadable BPF "Firmware" for vector drivers")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/drivers/vector_kern.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
index dc2feae789cb..63e5f108a6b9 100644
--- a/arch/um/drivers/vector_kern.c
+++ b/arch/um/drivers/vector_kern.c
@@ -141,7 +141,7 @@ static bool get_bpf_flash(struct arglist *def)
 
 	if (allow != NULL) {
 		if (kstrtoul(allow, 10, &result) == 0)
-			return (allow > 0);
+			return result > 0;
 	}
 	return false;
 }
-- 
2.44.0

[PATCH 4/5] um: vector: remove unused len variable/calculation

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The len variable is unused, so not needed, remove it.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/drivers/vector_kern.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c
index 63e5f108a6b9..4279793b11b7 100644
--- a/arch/um/drivers/vector_kern.c
+++ b/arch/um/drivers/vector_kern.c
@@ -712,11 +712,9 @@ static struct vector_device *find_device(int n)
 static int vector_parse(char *str, int *index_out, char **str_out,
 			char **error_out)
 {
-	int n, len, err;
+	int n, err;
 	char *start = str;
 
-	len = strlen(str);
-
 	while ((*str != ':') && (strlen(str) > 1))
 		str++;
 	if (*str != ':') {
-- 
2.44.0

[PATCH 5/5] um: process: remove unused 'n' variable

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

The return value of fn() wasn't used for a long time,
so no need to assign it to a variable, addressing a
W=1 warning.

This seems to be - with patches from others posted to
the list before - the last W=1 warning in arch/um/.

Fixes: 22e2430d60db ("x86, um: convert to saner kernel_execve() semantics")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 arch/um/kernel/process.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/um/kernel/process.c b/arch/um/kernel/process.c
index 30bf7739ed0f..d2134802f6a8 100644
--- a/arch/um/kernel/process.c
+++ b/arch/um/kernel/process.c
@@ -102,7 +102,7 @@ int get_current_pid(void)
  */
 void new_thread_handler(void)
 {
-	int (*fn)(void *), n;
+	int (*fn)(void *);
 	void *arg;
 
 	if (current->thread.prev_sched != NULL)
@@ -115,7 +115,7 @@ void new_thread_handler(void)
 	/*
 	 * callback returns only if the kernel thread execs a process
 	 */
-	n = fn(arg);
+	fn(arg);
 	userspace(&current->thread.regs.regs, current_thread_info()->aux_fp_regs);
 }
 
-- 
2.44.0

Re: [PATCH 1/5] um: signal: move pid variable where needed

[Tiwei Bie]

On 3/28/24 5:06 PM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> We have W=1 warnings on 64-bit because the pid is only
> used in branches on 32-bit; move it inside to get rid of
> the warnings.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  arch/x86/um/signal.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/um/signal.c b/arch/x86/um/signal.c
> index 263e1d08f216..16ff097e790d 100644
> --- a/arch/x86/um/signal.c
> +++ b/arch/x86/um/signal.c
> @@ -155,7 +155,7 @@ static int copy_sc_from_user(struct pt_regs *regs,
>  			     struct sigcontext __user *from)
>  {
>  	struct sigcontext sc;
> -	int err, pid;
> +	int err;
>  
>  	/* Always make any pending restarted system calls return -EINTR */
>  	current->restart_block.fn = do_no_restart_syscall;
> @@ -201,10 +201,10 @@ static int copy_sc_from_user(struct pt_regs *regs,
>  
>  #undef GETREG
>  
> -	pid = userspace_pid[current_thread_info()->cpu];
>  #ifdef CONFIG_X86_32
>  	if (have_fpx_regs) {
>  		struct user_fxsr_struct fpx;
> +		int pid = userspace_pid[current_thread_info()->cpu];

Tiny nit: might be better to move it before "struct user_fxsr_struct fpx;".
And it will be more consistent with the other change in this patch. :)

>  
>  		err = copy_from_user(&fpx,
>  			&((struct _fpstate __user *)sc.fpstate)->_fxsr_env[0],
> @@ -240,7 +240,7 @@ static int copy_sc_to_user(struct sigcontext __user *to,
>  {
>  	struct sigcontext sc;
>  	struct faultinfo * fi = &current->thread.arch.faultinfo;
> -	int err, pid;
> +	int err;
>  	memset(&sc, 0, sizeof(struct sigcontext));
>  
>  #define PUTREG(regno, regname) sc.regname = regs->regs.gp[HOST_##regno]
> @@ -288,10 +288,9 @@ static int copy_sc_to_user(struct sigcontext __user *to,
>  	if (err)
>  		return 1;
>  
> -	pid = userspace_pid[current_thread_info()->cpu];
> -
>  #ifdef CONFIG_X86_32
>  	if (have_fpx_regs) {
> +		int pid = userspace_pid[current_thread_info()->cpu];
>  		struct user_fxsr_struct fpx;
>  
>  		err = save_fpx_registers(pid, (unsigned long *) &fpx);

For the series, feel free to add my Reviewed-by: Tiwei Bie <tiwei.btw@antgroup.com>

Regards,
Tiwei