Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling
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 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] Staging: dgnc: Use goto for error handling
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
> ---
> 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
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] Staging: dgnc: Use goto for error handling
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
---
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))
+
