Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1563219

Re: [PATCH 3/8] powerpc/nvram: Move an assignment for the variable "ret" in dev_nvram_write()

Show all headers | View raw

On 01/19/2017 08:56 AM, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 19 Jan 2017 15:55:36 +0100
> 
> A local variable was set to an error code before a concrete error situation
> was detected. Thus move the corresponding assignment into an if branch
> to indicate a software failure there.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  arch/powerpc/kernel/nvram_64.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index cf839adf3aa7..dc90a0e9ad65 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -806,9 +806,10 @@ static ssize_t dev_nvram_write(struct file *file, const char __user *buf,
>  	if (!tmp)
>  		return -ENOMEM;
> 
> -	ret = -EFAULT;
> -	if (copy_from_user(tmp, buf, count))
> +	if (copy_from_user(tmp, buf, count)) {
> +		ret = -EFAULT;
>  		goto out;
> +	}
>
>  	ret = ppc_md.nvram_write(tmp, count, ppos);
> 

I think you really could have squashed patches 1-3 into a single patch
that returns directly after any failure. After this 3rd patch this is
now the only spot that branches to the "out" label. At this point you
might as well remove that label and move the kfree(tmp) call up and
return directly after the failure and at the nvram_write() call site
doing away completely with the "ret" variable.

Something like the following patch:

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index d5e2b83..eadb55c 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -789,37 +789,29 @@ static ssize_t dev_nvram_read(struct file *file,
char __user *buf,
 static ssize_t dev_nvram_write(struct file *file, const char __user *buf,
                          size_t count, loff_t *ppos)
 {
-       ssize_t ret;
-       char *tmp = NULL;
+       char *tmp;
        ssize_t size;

-       ret = -ENODEV;
        if (!ppc_md.nvram_size)
-               goto out;
+               return -ENODEV;

-       ret = 0;
        size = ppc_md.nvram_size();
        if (*ppos >= size || size < 0)
-               goto out;
+               return 0;

        count = min_t(size_t, count, size - *ppos);
        count = min(count, PAGE_SIZE);

-       ret = -ENOMEM;
        tmp = kmalloc(count, GFP_KERNEL);
        if (!tmp)
-               goto out;
-
-       ret = -EFAULT;
-       if (copy_from_user(tmp, buf, count))
-               goto out;
-
-       ret = ppc_md.nvram_write(tmp, count, ppos);
+               return -ENOMEM;

-out:
-       kfree(tmp);
-       return ret;
+       if (copy_from_user(tmp, buf, count)) {
+               kfree(tmp);
+               return -EFAULT;
+       }

+       return ppc_md.nvram_write(tmp, count, ppos);
 }

 static long dev_nvram_ioctl(struct file *file, unsigned int cmd,

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/8] PowerPC-NVRAM: Fine-tuning for some function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-19 18:00 +0100
  [PATCH 7/8] powerpc/nvram: Improve size determinations in three  functions SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-19 18:10 +0100
  [PATCH 3/8] powerpc/nvram: Move an assignment for the variable "ret"  in dev_nvram_write() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-19 18:10 +0100
    Re: [PATCH 3/8] powerpc/nvram: Move an assignment for the variable  "ret" in dev_nvram_write() Tyrel Datwyler <tyreld@linux.vnet.ibm.com> - 2017-01-20 01:30 +0100
      Re: powerpc/nvram: Move an assignment for the variable "ret" in  dev_nvram_write() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-20 08:10 +0100
        Re: powerpc/nvram: Move an assignment for the variable "ret" in  dev_nvram_write() Tyrel Datwyler <tyreld@linux.vnet.ibm.com> - 2017-01-20 22:00 +0100
  [PATCH 6/8] powerpc/nvram: Delete three error messages for a failed  memory allocation SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-19 18:10 +0100
  [PATCH 8/8] powerpc/nvram: Move an assignment for the variable "err"  in nvram_scan_partitions() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-19 18:40 +0100
  [PATCH 5/8] powerpc/nvram: Return directly after a failed kmalloc()  in dev_nvram_read() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-19 21:50 +0100

csiph-web