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

Groups > linux.kernel > #1564194 > unrolled thread

[PATCH 0/5] s390/pci: Fine-tuning for three function implementations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] s390/pci: Fine-tuning for three function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-21 19:10 +0100
    [PATCH 2/5] s390/pci: Return directly after a failed  clp_alloc_block() in clp_normal_command() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-21 19:20 +0100
    [PATCH 5/5] s390/pci: Move two assignments for the variable "ret" in  get_pfn() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-21 19:20 +0100
    [PATCH 4/5] s390/pci: Move an assignment for the variable "ret" in  s390_pci_mmio_write() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-21 19:20 +0100
      Re: [PATCH 4/5] s390/pci: Move an assignment for the variable "ret"  in s390_pci_mmio_write() Dan Carpenter <dan.carpenter@oracle.com> - 2017-01-23 10:00 +0100
    [PATCH 1/5] s390/pci: Return directly after a failed parameter  validation in clp_normal_command() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-21 19:20 +0100
    [PATCH 3/5] s390/pci: Move three assignments for the variable "rc" in  clp_normal_command() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-01-21 19:20 +0100
      Re: [PATCH 3/5] s390/pci: Move three assignments for the variable  "rc" in clp_normal_command() Christophe JAILLET <christophe.jaillet@wanadoo.fr> - 2017-01-22 09:00 +0100
    Re: [PATCH 0/5] s390/pci: Fine-tuning for three function  implementations Dan Carpenter <dan.carpenter@oracle.com> - 2017-01-23 10:00 +0100

#1564194 — [PATCH 0/5] s390/pci: Fine-tuning for three function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Jan 2017 18:58:18 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Return directly after a failed parameter validation in clp_normal_command()
  Return directly after a failed clp_alloc_block() in clp_normal_command()
  Move three assignments for the variable "rc" in clp_normal_command()
  Move an assignment for the variable "ret" in s390_pci_mmio_write()
  Move two assignments for the variable "ret" in get_pfn()

 arch/s390/pci/pci_clp.c  | 20 +++++++++-----------
 arch/s390/pci/pci_mmio.c | 19 ++++++++++---------
 2 files changed, 19 insertions(+), 20 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1564196 — [PATCH 2/5] s390/pci: Return directly after a failed clp_alloc_block() in clp_normal_command()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Jan 2017 17:56:40 +0100

* Return directly after a call of the function "clp_alloc_block"
  failed here.

* Delete the jump label "out" which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/s390/pci/pci_clp.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
index 4f5a1bf3b14b..28e13fa7a79c 100644
--- a/arch/s390/pci/pci_clp.c
+++ b/arch/s390/pci/pci_clp.c
@@ -517,10 +517,9 @@ static int clp_normal_command(struct clp_req *req)
 	if (req->lps != 0 && req->lps != 2)
 		return -EINVAL;
 
-	rc = -ENOMEM;
 	lpcb = clp_alloc_block(GFP_KERNEL);
 	if (!lpcb)
-		goto out;
+		return -ENOMEM;
 
 	rc = -EFAULT;
 	uptr = (void __force __user *)(unsigned long) req->data_p;
@@ -550,7 +549,6 @@ static int clp_normal_command(struct clp_req *req)
 
 out_free:
 	clp_free_block(lpcb);
-out:
 	return rc;
 }
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1564197 — [PATCH 5/5] s390/pci: Move two assignments for the variable "ret" in get_pfn()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Jan 2017 18:48:45 +0100

A local variable was set to an error code in two cases before a concrete
error situation was detected. Thus move the corresponding assignments into
if branches 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/s390/pci/pci_mmio.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
index f125858a77e9..400af13ef275 100644
--- a/arch/s390/pci/pci_mmio.c
+++ b/arch/s390/pci/pci_mmio.c
@@ -18,13 +18,15 @@ static long get_pfn(unsigned long user_addr, unsigned long access,
 	long ret;
 
 	down_read(&current->mm->mmap_sem);
-	ret = -EINVAL;
 	vma = find_vma(current->mm, user_addr);
-	if (!vma)
+	if (!vma) {
+		ret = -EINVAL;
 		goto out;
-	ret = -EACCES;
-	if (!(vma->vm_flags & access))
+	}
+	if (!(vma->vm_flags & access)) {
+		ret = -EACCES;
 		goto out;
+	}
 	ret = follow_pfn(vma, user_addr, pfn);
 out:
 	up_read(&current->mm->mmap_sem);
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1564199 — [PATCH 4/5] s390/pci: Move an assignment for the variable "ret" in s390_pci_mmio_write()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Jan 2017 18:35:00 +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.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/s390/pci/pci_mmio.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/s390/pci/pci_mmio.c b/arch/s390/pci/pci_mmio.c
index b1bb2b72302c..f125858a77e9 100644
--- a/arch/s390/pci/pci_mmio.c
+++ b/arch/s390/pci/pci_mmio.c
@@ -57,12 +57,11 @@ SYSCALL_DEFINE3(s390_pci_mmio_write, unsigned long, mmio_addr,
 		goto out;
 	io_addr = (void __iomem *)((pfn << PAGE_SHIFT) | (mmio_addr & ~PAGE_MASK));
 
-	ret = -EFAULT;
-	if ((unsigned long) io_addr < ZPCI_IOMAP_ADDR_BASE)
-		goto out;
-
-	if (copy_from_user(buf, user_buffer, length))
+	if ((unsigned long)io_addr < ZPCI_IOMAP_ADDR_BASE ||
+	    copy_from_user(buf, user_buffer, length)) {
+		ret = -EFAULT;
 		goto out;
+	}
 
 	ret = zpci_memcpy_toio(io_addr, buf, length);
 out:
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1564776 — Re: [PATCH 4/5] s390/pci: Move an assignment for the variable "ret" in s390_pci_mmio_write()

The original was more readable.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1564200 — [PATCH 1/5] s390/pci: Return directly after a failed parameter validation in clp_normal_command()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Jan 2017 17:49:00 +0100

Return directly after an inappropriate input parameter was detected.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/s390/pci/pci_clp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
index 1c3332ac1957..4f5a1bf3b14b 100644
--- a/arch/s390/pci/pci_clp.c
+++ b/arch/s390/pci/pci_clp.c
@@ -514,9 +514,8 @@ static int clp_normal_command(struct clp_req *req)
 	void __user *uptr;
 	int rc;
 
-	rc = -EINVAL;
 	if (req->lps != 0 && req->lps != 2)
-		goto out;
+		return -EINVAL;
 
 	rc = -ENOMEM;
 	lpcb = clp_alloc_block(GFP_KERNEL);
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1564202 — [PATCH 3/5] s390/pci: Move three assignments for the variable "rc" in clp_normal_command()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 21 Jan 2017 18:10:57 +0100

A local variable was set to an error code before a concrete error situation
was detected. Thus move the corresponding assignments into if branches
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/s390/pci/pci_clp.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c
index 28e13fa7a79c..bde0291cc724 100644
--- a/arch/s390/pci/pci_clp.c
+++ b/arch/s390/pci/pci_clp.c
@@ -521,14 +521,16 @@ static int clp_normal_command(struct clp_req *req)
 	if (!lpcb)
 		return -ENOMEM;
 
-	rc = -EFAULT;
 	uptr = (void __force __user *)(unsigned long) req->data_p;
-	if (copy_from_user(lpcb, uptr, PAGE_SIZE) != 0)
+	if (copy_from_user(lpcb, uptr, PAGE_SIZE) != 0) {
+		rc = -EFAULT;
 		goto out_free;
+	}
 
-	rc = -EINVAL;
-	if (lpcb->fmt != 0 || lpcb->reserved1 != 0 || lpcb->reserved2 != 0)
+	if (lpcb->fmt != 0 || lpcb->reserved1 != 0 || lpcb->reserved2 != 0) {
+		rc = -EINVAL;
 		goto out_free;
+	}
 
 	switch (req->lps) {
 	case 0:
@@ -541,9 +543,8 @@ static int clp_normal_command(struct clp_req *req)
 	if (rc)
 		goto out_free;
 
-	rc = -EFAULT;
 	if (copy_to_user(uptr, lpcb, PAGE_SIZE) != 0)
-		goto out_free;
+		rc = -EFAULT;
 
 	rc = 0;
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1564323 — Re: [PATCH 3/5] s390/pci: Move three assignments for the variable "rc" in clp_normal_command()

Le 21/01/2017 à 19:12, SF Markus Elfring a écrit :
> @@ -541,9 +543,8 @@ static int clp_normal_command(struct clp_req *req)
>   	if (rc)
>   		goto out_free;
>   
> -	rc = -EFAULT;
>   	if (copy_to_user(uptr, lpcb, PAGE_SIZE) != 0)
> -		goto out_free;
> +		rc = -EFAULT;
>   
>   	rc = 0;
>   

With this, we return 0 (success) on error.

CJ

[toc] | [prev] | [next] | [standalone]

#1564772 — Re: [PATCH 0/5] s390/pci: Fine-tuning for three function implementations

You should compile test your patches...  No one should have to explain
that.

regards,
dan carpenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web