[PATCH 2/3] Staging: dgnc: Use goto for error handling

[PATCH 2/3] Staging: dgnc: Use goto for error handling

[Quentin Lambert]

This patch introduces goto statments for error handling
and in cases where a lock needs to be released.

A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)

@candidates exists@
identifier f, label;
statement s;
position p1, p2, p3;
@@

  f@p1(...) {
  ...when any

    if@p2(...) {
    ...when any
      s

      return@p3 ...;
    }
  ...when any
  }

@good1 exists@
identifier candidates.f, candidates.label;
statement candidates.s;
position candidates.p1, candidates.p2;
@@

  f@p1(...) {
  ...when any

    if(...) {
    ...when any
      s
      return ...;
    }
    ...when any

    if@p2(...) {...}
  ...when any
 }

@depends on good1@
identifier candidates.f, candidates.label;
position candidates.p1, candidates.p3;
@@

   f@p1(...) {
   ...when any
*  return@p3 ...;
  }

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/staging/dgnc/dgnc_cls.c    | 19 +++++++------------
 drivers/staging/dgnc/dgnc_driver.c | 14 +++-----------
 drivers/staging/dgnc/dgnc_neo.c    | 30 ++++++++++++------------------
 3 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
index bedc522..02f19f1 100644
--- a/drivers/staging/dgnc/dgnc_cls.c
+++ b/drivers/staging/dgnc/dgnc_cls.c
@@ -1029,22 +1029,16 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
 	spin_lock_irqsave(&ch->ch_lock, flags);
 
 	/* No data to write to the UART */
-	if (ch->ch_w_tail == ch->ch_w_head) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if (ch->ch_w_tail == ch->ch_w_head)
+		goto exit_unlock;
 
 	/* If port is "stopped", don't send any data to the UART */
 	if ((ch->ch_flags & CH_FORCED_STOP) ||
-				 (ch->ch_flags & CH_BREAK_SENDING)) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+				 (ch->ch_flags & CH_BREAK_SENDING))
+		goto exit_unlock;
 
-	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
+		goto exit_unlock;
 
 	n = 32;
 
@@ -1094,6 +1088,7 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
 	if (len_written > 0)
 		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 
+exit_unlock:
 	spin_unlock_irqrestore(&ch->ch_lock, flags);
 }
 
diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
index b7fd2bf..9387a0e 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -572,30 +572,19 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 
 	rc = dgnc_tty_register(brd);
 	if (rc < 0) {
-		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't register tty devices (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
 		goto failed;
 	}
 
 	rc = dgnc_finalize_board_init(brd);
 	if (rc < 0) {
-		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't finalize board init (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
-
 		goto failed;
 	}
 
 	rc = dgnc_tty_init(brd);
 	if (rc < 0) {
-		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't init tty devices (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
-
 		goto failed;
 	}
 
@@ -629,6 +618,9 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 	return 0;
 
 failed:
+	dgnc_tty_uninit(brd);
+	brd->state = BOARD_FAILED;
+	brd->dpastatus = BD_NOFEP;
 
 	return -ENXIO;
 
diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index 1268aa9..4deae2d 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -1436,16 +1436,13 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 	spin_lock_irqsave(&ch->ch_lock, flags);
 
 	/* No data to write to the UART */
-	if (ch->ch_w_tail == ch->ch_w_head) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if (ch->ch_w_tail == ch->ch_w_head)
+		goto exit_unlock;
 
 	/* If port is "stopped", don't send any data to the UART */
-	if ((ch->ch_flags & CH_FORCED_STOP) || (ch->ch_flags & CH_BREAK_SENDING)) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if ((ch->ch_flags & CH_FORCED_STOP) ||
+		 (ch->ch_flags & CH_BREAK_SENDING))
+		goto exit_unlock;
 
 	/*
 	 * If FIFOs are disabled. Send data directly to txrx register
@@ -1486,27 +1483,23 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 			ch->ch_w_tail &= WQUEUEMASK;
 			ch->ch_txcount++;
 		}
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
+
+		goto exit_unlock;
 	}
 
 	/*
 	 * We have to do it this way, because of the EXAR TXFIFO count bug.
 	 */
 	if ((ch->ch_bd->dvid & 0xf0) < UART_XR17E158_DVID) {
-		if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			return;
-		}
+		if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
+			goto exit_unlock;
 
 		len_written = 0;
 
 		n = readb(&ch->ch_neo_uart->tfifo);
 
-		if ((unsigned int) n > ch->ch_t_tlevel) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			return;
-		}
+		if ((unsigned int) n > ch->ch_t_tlevel)
+			goto exit_unlock;
 
 		n = UART_17158_TX_FIFOSIZE - ch->ch_t_tlevel;
 	} else {
@@ -1570,6 +1563,7 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 	}
 
+exit_unlock:
 	spin_unlock_irqrestore(&ch->ch_lock, flags);
 }
 
-- 
1.9.1

[PATCH 2/3] Staging: dgnc: Use goto for error handling

[Quentin Lambert]

This patch introduces goto statments for error handling
and in cases where a lock needs to be released.

A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr)

@candidates exists@
identifier f, label;
statement s;
position p1, p2, p3;
@@

  f@p1(...) {
  ...when any

    if@p2(...) {
    ...when any
      s

      return@p3 ...;
    }
  ...when any
  }

@good1 exists@
identifier candidates.f, candidates.label;
statement candidates.s;
position candidates.p1, candidates.p2;
@@

  f@p1(...) {
  ...when any

    if(...) {
    ...when any
      s
      return ...;
    }
    ...when any

    if@p2(...) {...}
  ...when any
 }

@depends on good1@
identifier candidates.f, candidates.label;
position candidates.p1, candidates.p3;
@@

   f@p1(...) {
   ...when any
*  return@p3 ...;
  }

Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
 drivers/staging/dgnc/dgnc_cls.c    | 19 +++++++------------
 drivers/staging/dgnc/dgnc_driver.c | 14 +++-----------
 drivers/staging/dgnc/dgnc_neo.c    | 30 ++++++++++++------------------
 3 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
index bedc522..02f19f1 100644
--- a/drivers/staging/dgnc/dgnc_cls.c
+++ b/drivers/staging/dgnc/dgnc_cls.c
@@ -1029,22 +1029,16 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
 	spin_lock_irqsave(&ch->ch_lock, flags);
 
 	/* No data to write to the UART */
-	if (ch->ch_w_tail = ch->ch_w_head) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if (ch->ch_w_tail = ch->ch_w_head)
+		goto exit_unlock;
 
 	/* If port is "stopped", don't send any data to the UART */
 	if ((ch->ch_flags & CH_FORCED_STOP) ||
-				 (ch->ch_flags & CH_BREAK_SENDING)) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+				 (ch->ch_flags & CH_BREAK_SENDING))
+		goto exit_unlock;
 
-	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
+		goto exit_unlock;
 
 	n = 32;
 
@@ -1094,6 +1088,7 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
 	if (len_written > 0)
 		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 
+exit_unlock:
 	spin_unlock_irqrestore(&ch->ch_lock, flags);
 }
 
diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
index b7fd2bf..9387a0e 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -572,30 +572,19 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 
 	rc = dgnc_tty_register(brd);
 	if (rc < 0) {
-		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't register tty devices (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
 		goto failed;
 	}
 
 	rc = dgnc_finalize_board_init(brd);
 	if (rc < 0) {
-		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't finalize board init (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
-
 		goto failed;
 	}
 
 	rc = dgnc_tty_init(brd);
 	if (rc < 0) {
-		dgnc_tty_uninit(brd);
 		pr_err(DRVSTR ": Can't init tty devices (%d)\n", rc);
-		brd->state = BOARD_FAILED;
-		brd->dpastatus = BD_NOFEP;
-
 		goto failed;
 	}
 
@@ -629,6 +618,9 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
 	return 0;
 
 failed:
+	dgnc_tty_uninit(brd);
+	brd->state = BOARD_FAILED;
+	brd->dpastatus = BD_NOFEP;
 
 	return -ENXIO;
 
diff --git a/drivers/staging/dgnc/dgnc_neo.c b/drivers/staging/dgnc/dgnc_neo.c
index 1268aa9..4deae2d 100644
--- a/drivers/staging/dgnc/dgnc_neo.c
+++ b/drivers/staging/dgnc/dgnc_neo.c
@@ -1436,16 +1436,13 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 	spin_lock_irqsave(&ch->ch_lock, flags);
 
 	/* No data to write to the UART */
-	if (ch->ch_w_tail = ch->ch_w_head) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if (ch->ch_w_tail = ch->ch_w_head)
+		goto exit_unlock;
 
 	/* If port is "stopped", don't send any data to the UART */
-	if ((ch->ch_flags & CH_FORCED_STOP) || (ch->ch_flags & CH_BREAK_SENDING)) {
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
-	}
+	if ((ch->ch_flags & CH_FORCED_STOP) ||
+		 (ch->ch_flags & CH_BREAK_SENDING))
+		goto exit_unlock;
 
 	/*
 	 * If FIFOs are disabled. Send data directly to txrx register
@@ -1486,27 +1483,23 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 			ch->ch_w_tail &= WQUEUEMASK;
 			ch->ch_txcount++;
 		}
-		spin_unlock_irqrestore(&ch->ch_lock, flags);
-		return;
+
+		goto exit_unlock;
 	}
 
 	/*
 	 * We have to do it this way, because of the EXAR TXFIFO count bug.
 	 */
 	if ((ch->ch_bd->dvid & 0xf0) < UART_XR17E158_DVID) {
-		if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			return;
-		}
+		if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
+			goto exit_unlock;
 
 		len_written = 0;
 
 		n = readb(&ch->ch_neo_uart->tfifo);
 
-		if ((unsigned int) n > ch->ch_t_tlevel) {
-			spin_unlock_irqrestore(&ch->ch_lock, flags);
-			return;
-		}
+		if ((unsigned int) n > ch->ch_t_tlevel)
+			goto exit_unlock;
 
 		n = UART_17158_TX_FIFOSIZE - ch->ch_t_tlevel;
 	} else {
@@ -1570,6 +1563,7 @@ static void neo_copy_data_from_queue_to_uart(struct channel_t *ch)
 		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
 	}
 
+exit_unlock:
 	spin_unlock_irqrestore(&ch->ch_lock, flags);
 }
 
-- 
1.9.1

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Giedrius Statkevičius]

On 2015.03.11 16:04, Quentin Lambert wrote:
> This patch introduces goto statments for error handling
> and in cases where a lock needs to be released.
> 
> A simplified version of the semantic patch that finds this problem is as
> follows: (http://coccinelle.lip6.fr)
> 
> @candidates exists@
> identifier f, label;
> statement s;
> position p1, p2, p3;
> @@
> 
>   f@p1(...) {
>   ...when any
> 
>     if@p2(...) {
>     ...when any
>       s
> 
>       return@p3 ...;
>     }
>   ...when any
>   }
> 
> @good1 exists@
> identifier candidates.f, candidates.label;
> statement candidates.s;
> position candidates.p1, candidates.p2;
> @@
> 
>   f@p1(...) {
>   ...when any
> 
>     if(...) {
>     ...when any
>       s
>       return ...;
>     }
>     ...when any
> 
>     if@p2(...) {...}
>   ...when any
>  }
> 
> @depends on good1@
> identifier candidates.f, candidates.label;
> position candidates.p1, candidates.p3;
> @@
> 
>    f@p1(...) {
>    ...when any
> *  return@p3 ...;
>   }
> 
> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_cls.c    | 19 +++++++------------
>  drivers/staging/dgnc/dgnc_driver.c | 14 +++-----------
>  drivers/staging/dgnc/dgnc_neo.c    | 30 ++++++++++++------------------
>  3 files changed, 22 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index bedc522..02f19f1 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -1029,22 +1029,16 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
>  	spin_lock_irqsave(&ch->ch_lock, flags);
>  
>  	/* No data to write to the UART */
> -	if (ch->ch_w_tail == ch->ch_w_head) {
> -		spin_unlock_irqrestore(&ch->ch_lock, flags);
> -		return;
> -	}
> +	if (ch->ch_w_tail == ch->ch_w_head)
> +		goto exit_unlock;
>  
>  	/* If port is "stopped", don't send any data to the UART */
>  	if ((ch->ch_flags & CH_FORCED_STOP) ||
> -				 (ch->ch_flags & CH_BREAK_SENDING)) {
> -		spin_unlock_irqrestore(&ch->ch_lock, flags);
> -		return;
> -	}
> +				 (ch->ch_flags & CH_BREAK_SENDING))
> +		goto exit_unlock;
>  
> -	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
> -		spin_unlock_irqrestore(&ch->ch_lock, flags);
> -		return;
> -	}
> +	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
> +		goto exit_unlock;
>  
>  	n = 32;
>  
> @@ -1094,6 +1088,7 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
>  	if (len_written > 0)
>  		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
>  
> +exit_unlock:
>  	spin_unlock_irqrestore(&ch->ch_lock, flags);
>  }
>  
> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
> index b7fd2bf..9387a0e 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -572,30 +572,19 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>  
>  	rc = dgnc_tty_register(brd);
>  	if (rc < 0) {
> -		dgnc_tty_uninit(brd);
>  		pr_err(DRVSTR ": Can't register tty devices (%d)\n", rc);
> -		brd->state = BOARD_FAILED;
> -		brd->dpastatus = BD_NOFEP;
>  		goto failed;
>  	}
>  
>  	rc = dgnc_finalize_board_init(brd);
>  	if (rc < 0) {
> -		dgnc_tty_uninit(brd);
>  		pr_err(DRVSTR ": Can't finalize board init (%d)\n", rc);
> -		brd->state = BOARD_FAILED;
> -		brd->dpastatus = BD_NOFEP;
> -
>  		goto failed;
>  	}
>  
>  	rc = dgnc_tty_init(brd);
>  	if (rc < 0) {
> -		dgnc_tty_uninit(brd);
>  		pr_err(DRVSTR ": Can't init tty devices (%d)\n", rc);
> -		brd->state = BOARD_FAILED;
> -		brd->dpastatus = BD_NOFEP;
> -
>  		goto failed;
>  	}
>  
> @@ -629,6 +618,9 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>  	return 0;
>  
>  failed:
> +	dgnc_tty_uninit(brd);
> +	brd->state = BOARD_FAILED;
> +	brd->dpastatus = BD_NOFEP;
You'll probably have to remake this patch without this change because a
patch I've submitted a few days ago completely removes these and 2
other ones depend on it. It's still not in staging-testing for some reason :(

-- 
Thanks,
Giedrius

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Giedrius Statkevičius]

On 2015.03.11 16:04, Quentin Lambert wrote:
> This patch introduces goto statments for error handling
> and in cases where a lock needs to be released.
> 
> A simplified version of the semantic patch that finds this problem is as
> follows: (http://coccinelle.lip6.fr)
> 
> @candidates exists@
> identifier f, label;
> statement s;
> position p1, p2, p3;
> @@
> 
>   f@p1(...) {
>   ...when any
> 
>     if@p2(...) {
>     ...when any
>       s
> 
>       return@p3 ...;
>     }
>   ...when any
>   }
> 
> @good1 exists@
> identifier candidates.f, candidates.label;
> statement candidates.s;
> position candidates.p1, candidates.p2;
> @@
> 
>   f@p1(...) {
>   ...when any
> 
>     if(...) {
>     ...when any
>       s
>       return ...;
>     }
>     ...when any
> 
>     if@p2(...) {...}
>   ...when any
>  }
> 
> @depends on good1@
> identifier candidates.f, candidates.label;
> position candidates.p1, candidates.p3;
> @@
> 
>    f@p1(...) {
>    ...when any
> *  return@p3 ...;
>   }
> 
> Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
> ---
>  drivers/staging/dgnc/dgnc_cls.c    | 19 +++++++------------
>  drivers/staging/dgnc/dgnc_driver.c | 14 +++-----------
>  drivers/staging/dgnc/dgnc_neo.c    | 30 ++++++++++++------------------
>  3 files changed, 22 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/staging/dgnc/dgnc_cls.c b/drivers/staging/dgnc/dgnc_cls.c
> index bedc522..02f19f1 100644
> --- a/drivers/staging/dgnc/dgnc_cls.c
> +++ b/drivers/staging/dgnc/dgnc_cls.c
> @@ -1029,22 +1029,16 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
>  	spin_lock_irqsave(&ch->ch_lock, flags);
>  
>  	/* No data to write to the UART */
> -	if (ch->ch_w_tail = ch->ch_w_head) {
> -		spin_unlock_irqrestore(&ch->ch_lock, flags);
> -		return;
> -	}
> +	if (ch->ch_w_tail = ch->ch_w_head)
> +		goto exit_unlock;
>  
>  	/* If port is "stopped", don't send any data to the UART */
>  	if ((ch->ch_flags & CH_FORCED_STOP) ||
> -				 (ch->ch_flags & CH_BREAK_SENDING)) {
> -		spin_unlock_irqrestore(&ch->ch_lock, flags);
> -		return;
> -	}
> +				 (ch->ch_flags & CH_BREAK_SENDING))
> +		goto exit_unlock;
>  
> -	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM))) {
> -		spin_unlock_irqrestore(&ch->ch_lock, flags);
> -		return;
> -	}
> +	if (!(ch->ch_flags & (CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM)))
> +		goto exit_unlock;
>  
>  	n = 32;
>  
> @@ -1094,6 +1088,7 @@ static void cls_copy_data_from_queue_to_uart(struct channel_t *ch)
>  	if (len_written > 0)
>  		ch->ch_flags &= ~(CH_TX_FIFO_EMPTY | CH_TX_FIFO_LWM);
>  
> +exit_unlock:
>  	spin_unlock_irqrestore(&ch->ch_lock, flags);
>  }
>  
> diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
> index b7fd2bf..9387a0e 100644
> --- a/drivers/staging/dgnc/dgnc_driver.c
> +++ b/drivers/staging/dgnc/dgnc_driver.c
> @@ -572,30 +572,19 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>  
>  	rc = dgnc_tty_register(brd);
>  	if (rc < 0) {
> -		dgnc_tty_uninit(brd);
>  		pr_err(DRVSTR ": Can't register tty devices (%d)\n", rc);
> -		brd->state = BOARD_FAILED;
> -		brd->dpastatus = BD_NOFEP;
>  		goto failed;
>  	}
>  
>  	rc = dgnc_finalize_board_init(brd);
>  	if (rc < 0) {
> -		dgnc_tty_uninit(brd);
>  		pr_err(DRVSTR ": Can't finalize board init (%d)\n", rc);
> -		brd->state = BOARD_FAILED;
> -		brd->dpastatus = BD_NOFEP;
> -
>  		goto failed;
>  	}
>  
>  	rc = dgnc_tty_init(brd);
>  	if (rc < 0) {
> -		dgnc_tty_uninit(brd);
>  		pr_err(DRVSTR ": Can't init tty devices (%d)\n", rc);
> -		brd->state = BOARD_FAILED;
> -		brd->dpastatus = BD_NOFEP;
> -
>  		goto failed;
>  	}
>  
> @@ -629,6 +618,9 @@ static int dgnc_found_board(struct pci_dev *pdev, int id)
>  	return 0;
>  
>  failed:
> +	dgnc_tty_uninit(brd);
> +	brd->state = BOARD_FAILED;
> +	brd->dpastatus = BD_NOFEP;
You'll probably have to remake this patch without this change because a
patch I've submitted a few days ago completely removes these and 2
other ones depend on it. It's still not in staging-testing for some reason :(

-- 
Thanks,
Giedrius

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Dan Carpenter]

On Wed, Mar 11, 2015 at 06:37:30PM +0200, Giedrius Statkevičius wrote:
> It's still not in staging-testing for some reason :(
> 

It can take several weeks to get merged.  Relax.

regards,
dan carpenter

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Dan Carpenter]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 403 bytes --]

On Wed, Mar 11, 2015 at 06:37:30PM +0200, Giedrius Statkevičius wrote:
> It's still not in staging-testing for some reason :(
> 

It can take several weeks to get merged.  Relax.

regards,
dan carpenter

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

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Quentin Lambert]

On 12/03/2015 10:27, Dan Carpenter wrote:
> On Wed, Mar 11, 2015 at 06:37:30PM +0200, Giedrius Statkevičius wrote:
>> It's still not in staging-testing for some reason :(
>>
> It can take several weeks to get merged.  Relax.
>
> regards,
> dan carpenter
>
What should i do concerning that ?
I need to send a second version of this patch anyway to fix the
fact that i inverted 2 statements, namely:

+exit_unlock:
+	if (ld)
+		tty_ldisc_deref(ld);
+	spin_unlock_irqrestore(&ch->ch_lock, flags);

should be

+exit_unlock:
+	spin_unlock_irqrestore(&ch->ch_lock, flags);
+	if (ld)
+		tty_ldisc_deref(ld);

Should I wait these several weeks to send the second version or should
I send a second version fixing this issue a wait for someone to ask me
to solve the conflict ?

Quentin

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Quentin Lambert]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 978 bytes --]



On 12/03/2015 10:27, Dan Carpenter wrote:
> On Wed, Mar 11, 2015 at 06:37:30PM +0200, Giedrius Statkevičius wrote:
>> It's still not in staging-testing for some reason :(
>>
> It can take several weeks to get merged.  Relax.
>
> regards,
> dan carpenter
>
What should i do concerning that ?
I need to send a second version of this patch anyway to fix the
fact that i inverted 2 statements, namely:

+exit_unlock:
+	if (ld)
+		tty_ldisc_deref(ld);
+	spin_unlock_irqrestore(&ch->ch_lock, flags);

should be

+exit_unlock:
+	spin_unlock_irqrestore(&ch->ch_lock, flags);
+	if (ld)
+		tty_ldisc_deref(ld);

Should I wait these several weeks to send the second version or should
I send a second version fixing this issue a wait for someone to ask me
to solve the conflict ?

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

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Dan Carpenter]

On Thu, Mar 12, 2015 at 10:32:40AM +0100, Quentin Lambert wrote:
> Should I wait these several weeks to send the second version or should
> I send a second version fixing this issue a wait for someone to ask me
> to solve the conflict ?

Greg applies patches in the order they come.  It's normally straight
forward which ones are going to be merged and which ones are not.  So
you could just apply Giedrius's patch and put a note in the --- cut off
the it depends on the earlier patch.

If the patch doesn't apply, Greg won't take any time to figure out why.
You'll just get a note to redo it.

If you two plan on working on the same driver for a while then you could
coordinate your patches and send them as a patchset.

regards,
dan carpenter

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Dan Carpenter]

On Thu, Mar 12, 2015 at 10:32:40AM +0100, Quentin Lambert wrote:
> Should I wait these several weeks to send the second version or should
> I send a second version fixing this issue a wait for someone to ask me
> to solve the conflict ?

Greg applies patches in the order they come.  It's normally straight
forward which ones are going to be merged and which ones are not.  So
you could just apply Giedrius's patch and put a note in the --- cut off
the it depends on the earlier patch.

If the patch doesn't apply, Greg won't take any time to figure out why.
You'll just get a note to redo it.

If you two plan on working on the same driver for a while then you could
coordinate your patches and send them as a patchset.

regards,
dan carpenter

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Greg Kroah-Hartman]

On Thu, Mar 12, 2015 at 10:32:40AM +0100, Quentin Lambert wrote:
> 
> 
> On 12/03/2015 10:27, Dan Carpenter wrote:
> >On Wed, Mar 11, 2015 at 06:37:30PM +0200, Giedrius Statkevičius wrote:
> >>It's still not in staging-testing for some reason :(
> >>
> >It can take several weeks to get merged.  Relax.
> >
> >regards,
> >dan carpenter
> >
> What should i do concerning that ?
> I need to send a second version of this patch anyway to fix the
> fact that i inverted 2 statements, namely:
> 
> +exit_unlock:
> +	if (ld)
> +		tty_ldisc_deref(ld);
> +	spin_unlock_irqrestore(&ch->ch_lock, flags);
> 
> should be
> 
> +exit_unlock:
> +	spin_unlock_irqrestore(&ch->ch_lock, flags);
> +	if (ld)
> +		tty_ldisc_deref(ld);
> 
> Should I wait these several weeks to send the second version or should
> I send a second version fixing this issue a wait for someone to ask me
> to solve the conflict ?

Just wait a bit, I should catch up on patches in this area soon, and I
will let everyone know if patches do not apply.

thanks,

greg k-h

Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling

[Greg Kroah-Hartman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1228 bytes --]

On Thu, Mar 12, 2015 at 10:32:40AM +0100, Quentin Lambert wrote:
> 
> 
> On 12/03/2015 10:27, Dan Carpenter wrote:
> >On Wed, Mar 11, 2015 at 06:37:30PM +0200, Giedrius Statkevičius wrote:
> >>It's still not in staging-testing for some reason :(
> >>
> >It can take several weeks to get merged.  Relax.
> >
> >regards,
> >dan carpenter
> >
> What should i do concerning that ?
> I need to send a second version of this patch anyway to fix the
> fact that i inverted 2 statements, namely:
> 
> +exit_unlock:
> +	if (ld)
> +		tty_ldisc_deref(ld);
> +	spin_unlock_irqrestore(&ch->ch_lock, flags);
> 
> should be
> 
> +exit_unlock:
> +	spin_unlock_irqrestore(&ch->ch_lock, flags);
> +	if (ld)
> +		tty_ldisc_deref(ld);
> 
> Should I wait these several weeks to send the second version or should
> I send a second version fixing this issue a wait for someone to ask me
> to solve the conflict ?

Just wait a bit, I should catch up on patches in this area soon, and I
will let everyone know if patches do not apply.

thanks,

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