Re: [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch)

2014-07-20 Thread Corentin LABBE 
Le 19/07/2014 20:00, Joe Perches a écrit :
 (Adding Mark Allyn and Jayant Mangalampalli)
 
 Is this still project still active?

I do not know

 
 On Sat, 2014-07-19 at 19:34 +0200, LABBE Corentin wrote:
 Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
 ---
  drivers/staging/sep/sep_main.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

 diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
 index 177e4b9..1580d95f 100644
 --- a/drivers/staging/sep/sep_main.c
 +++ b/drivers/staging/sep/sep_main.c
 @@ -2881,12 +2881,11 @@ static int sep_free_dma_tables_and_dcb(struct 
 sep_device *sep, bool isapplet,
  if (is_kva) {
  error = -ENODEV;
  break;
 -} else {
 -error_temp = copy_to_user(
 +}
 +error_temp = copy_to_user(
  (void __user *)tail_pt,
  dcb_table_ptr-tail_data,
  dcb_table_ptr-tail_data_size);
 -}
  if (error_temp) {
  /* Release the DMA resource */
  error = -EFAULT;
 
 It'd be probably be better to rewrite the code to unindent
 a level by using continue.  Something like below:
 
 btw:
 
 the is_kva test looks very odd and should probably be
 moved outside the loop.
 
 pt_hold should probably be void * not unsigned long
 as it loses high order bits on x86-32.
 
 definition:
   aligned_u64 out_vr_tail_pt;
 use:
 + pt_hold = (unsigned long)dcb_table_ptr-
 + out_vr_tail_pt;
 

As I said in the introduction email, I have done thoses patch for the Eudyptula 
challenge,
since I have not the hardware needed by this driver I cannot modify beyond 
simple style changes without testing

Regards


 ---
  drivers/staging/sep/sep_main.c | 37 +
  1 file changed, 17 insertions(+), 20 deletions(-)
 
 diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
 index 75ca15e..24b4a54 100644
 --- a/drivers/staging/sep/sep_main.c
 +++ b/drivers/staging/sep/sep_main.c
 @@ -2871,26 +2871,23 @@ static int sep_free_dma_tables_and_dcb(struct 
 sep_device *sep, bool isapplet,
* Go over each DCB and see if
* tail pointer must be updated
*/
 - for (i = 0; i  (*dma_ctx)-nr_dcb_creat;
 -  i++, dcb_table_ptr++) {
 - if (dcb_table_ptr-out_vr_tail_pt) {
 - pt_hold = (unsigned long)dcb_table_ptr-
 - out_vr_tail_pt;
 - tail_pt = (void *)pt_hold;
 - if (is_kva) {
 - error = -ENODEV;
 - break;
 - } else {
 - error_temp = copy_to_user(
 - (void __user *)tail_pt,
 - dcb_table_ptr-tail_data,
 - dcb_table_ptr-tail_data_size);
 - }
 - if (error_temp) {
 - /* Release the DMA resource */
 - error = -EFAULT;
 - break;
 - }
 + for (i = 0; i  (*dma_ctx)-nr_dcb_creat; i++, dcb_table_ptr++) 
 {
 + if (!dcb_table_ptr-out_vr_tail_pt)
 + continue;
 + pt_hold = (unsigned long)dcb_table_ptr-
 + out_vr_tail_pt;
 + tail_pt = (void *)pt_hold;
 + if (is_kva) {
 + error = -ENODEV;
 + break;
 + }
 + error_temp = copy_to_user((void __user *)tail_pt,
 +   dcb_table_ptr-tail_data,
 +   
 dcb_table_ptr-tail_data_size);
 + if (error_temp) {
 + /* Release the DMA resource */
 + error = -EFAULT;
 + break;
   }
   }
   }
 
 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch)

2014-07-19 Thread LABBE Corentin 
Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
---
 drivers/staging/sep/sep_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 177e4b9..1580d95f 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -2881,12 +2881,11 @@ static int sep_free_dma_tables_and_dcb(struct 
sep_device *sep, bool isapplet,
if (is_kva) {
error = -ENODEV;
break;
-   } else {
-   error_temp = copy_to_user(
+   }
+   error_temp = copy_to_user(
(void __user *)tail_pt,
dcb_table_ptr-tail_data,
dcb_table_ptr-tail_data_size);
-   }
if (error_temp) {
/* Release the DMA resource */
error = -EFAULT;
-- 
1.8.5.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/4] staging: sep: No else is necessary after a break (reported by checkpatch)

2014-07-19 Thread Joe Perches 
(Adding Mark Allyn and Jayant Mangalampalli)

Is this still project still active?

On Sat, 2014-07-19 at 19:34 +0200, LABBE Corentin wrote:
 Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
 ---
  drivers/staging/sep/sep_main.c | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
 index 177e4b9..1580d95f 100644
 --- a/drivers/staging/sep/sep_main.c
 +++ b/drivers/staging/sep/sep_main.c
 @@ -2881,12 +2881,11 @@ static int sep_free_dma_tables_and_dcb(struct 
 sep_device *sep, bool isapplet,
   if (is_kva) {
   error = -ENODEV;
   break;
 - } else {
 - error_temp = copy_to_user(
 + }
 + error_temp = copy_to_user(
   (void __user *)tail_pt,
   dcb_table_ptr-tail_data,
   dcb_table_ptr-tail_data_size);
 - }
   if (error_temp) {
   /* Release the DMA resource */
   error = -EFAULT;

It'd be probably be better to rewrite the code to unindent
a level by using continue.  Something like below:

btw:

the is_kva test looks very odd and should probably be
moved outside the loop.

pt_hold should probably be void * not unsigned long
as it loses high order bits on x86-32.

definition:
aligned_u64 out_vr_tail_pt;
use:
+   pt_hold = (unsigned long)dcb_table_ptr-
+   out_vr_tail_pt;

---
 drivers/staging/sep/sep_main.c | 37 +
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/sep/sep_main.c b/drivers/staging/sep/sep_main.c
index 75ca15e..24b4a54 100644
--- a/drivers/staging/sep/sep_main.c
+++ b/drivers/staging/sep/sep_main.c
@@ -2871,26 +2871,23 @@ static int sep_free_dma_tables_and_dcb(struct 
sep_device *sep, bool isapplet,
 * Go over each DCB and see if
 * tail pointer must be updated
 */
-   for (i = 0; i  (*dma_ctx)-nr_dcb_creat;
-i++, dcb_table_ptr++) {
-   if (dcb_table_ptr-out_vr_tail_pt) {
-   pt_hold = (unsigned long)dcb_table_ptr-
-   out_vr_tail_pt;
-   tail_pt = (void *)pt_hold;
-   if (is_kva) {
-   error = -ENODEV;
-   break;
-   } else {
-   error_temp = copy_to_user(
-   (void __user *)tail_pt,
-   dcb_table_ptr-tail_data,
-   dcb_table_ptr-tail_data_size);
-   }
-   if (error_temp) {
-   /* Release the DMA resource */
-   error = -EFAULT;
-   break;
-   }
+   for (i = 0; i  (*dma_ctx)-nr_dcb_creat; i++, dcb_table_ptr++) 
{
+   if (!dcb_table_ptr-out_vr_tail_pt)
+   continue;
+   pt_hold = (unsigned long)dcb_table_ptr-
+   out_vr_tail_pt;
+   tail_pt = (void *)pt_hold;
+   if (is_kva) {
+   error = -ENODEV;
+   break;
+   }
+   error_temp = copy_to_user((void __user *)tail_pt,
+ dcb_table_ptr-tail_data,
+ 
dcb_table_ptr-tail_data_size);
+   if (error_temp) {
+   /* Release the DMA resource */
+   error = -EFAULT;
+   break;
}
}
}


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel