Re: [PATCH 1/3] ALSA: bebob: Use common error handling code in snd_bebob_stream_start_duplex()

[Takashi Sakamoto]

Hi,

On Sep 6 2017 19:22, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Wed, 6 Sep 2017 11:40:53 +0200

Add jump targets so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
  sound/firewire/bebob/bebob_stream.c | 21 ++---
  1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c 
b/sound/firewire/bebob/bebob_stream.c
index 4d3034a68bdf..bc9e42b6368e 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
...
@@ -666,9 +661,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
if (err < 0) {
dev_err(>unit->device,
"fail to run AMDTP slave stream:%d\n", err);
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
-   goto end;
+   goto stop_rx_stream;
}
  
  		/* wait first callback */


After the above code block, we have below code block.

658 /* start slave if needed */
659 if (!amdtp_stream_running(>tx_stream)) {
660 err = start_stream(bebob, >tx_stream, rate);
661 if (err < 0) {
662 dev_err(>unit->device,
663 "fail to run AMDTP slave stream:%d\n", err);
664 goto stop_rx_stream;
665 }
666
667 /* wait first callback */
668 if (!amdtp_stream_wait_callback(>tx_stream,
669 CALLBACK_TIMEOUT)) {
670 amdtp_stream_stop(>tx_stream);
671 amdtp_stream_stop(>rx_stream);
672 break_both_connections(bebob);
673 err = -ETIMEDOUT;
674 }
675 }

I think it better to apply your solution too in the above to keep code 
consistency.



@@ -682,6 +675,12 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
}
  end:
return err;
+
+stop_rx_stream:
+   amdtp_stream_stop(>rx_stream);
+break_connections:
+   break_both_connections(bebob);
+   return err;
  }
  
  void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob)


For the other patches, I can find no merit to apply except for reduction 
of the number of characters included in the file.



Thanks

Takashi Sakamoto

Re: [PATCH 1/3] ALSA: bebob: Use common error handling code in snd_bebob_stream_start_duplex()

[Takashi Sakamoto]

Hi,

On Sep 6 2017 19:22, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Wed, 6 Sep 2017 11:40:53 +0200

Add jump targets so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
  sound/firewire/bebob/bebob_stream.c | 21 ++---
  1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c 
b/sound/firewire/bebob/bebob_stream.c
index 4d3034a68bdf..bc9e42b6368e 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
...
@@ -666,9 +661,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
if (err < 0) {
dev_err(>unit->device,
"fail to run AMDTP slave stream:%d\n", err);
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
-   goto end;
+   goto stop_rx_stream;
}
  
  		/* wait first callback */


After the above code block, we have below code block.

658 /* start slave if needed */
659 if (!amdtp_stream_running(>tx_stream)) {
660 err = start_stream(bebob, >tx_stream, rate);
661 if (err < 0) {
662 dev_err(>unit->device,
663 "fail to run AMDTP slave stream:%d\n", err);
664 goto stop_rx_stream;
665 }
666
667 /* wait first callback */
668 if (!amdtp_stream_wait_callback(>tx_stream,
669 CALLBACK_TIMEOUT)) {
670 amdtp_stream_stop(>tx_stream);
671 amdtp_stream_stop(>rx_stream);
672 break_both_connections(bebob);
673 err = -ETIMEDOUT;
674 }
675 }

I think it better to apply your solution too in the above to keep code 
consistency.



@@ -682,6 +675,12 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
}
  end:
return err;
+
+stop_rx_stream:
+   amdtp_stream_stop(>rx_stream);
+break_connections:
+   break_both_connections(bebob);
+   return err;
  }
  
  void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob)


For the other patches, I can find no merit to apply except for reduction 
of the number of characters included in the file.



Thanks

Takashi Sakamoto

[PATCH 1/3] ALSA: bebob: Use common error handling code in snd_bebob_stream_start_duplex()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 6 Sep 2017 11:40:53 +0200

Add jump targets so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 sound/firewire/bebob/bebob_stream.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c 
b/sound/firewire/bebob/bebob_stream.c
index 4d3034a68bdf..bc9e42b6368e 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -629,8 +629,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
if (err < 0) {
dev_err(>unit->device,
"fail to run AMDTP master stream:%d\n", err);
-   break_both_connections(bebob);
-   goto end;
+   goto break_connections;
}
 
/*
@@ -644,19 +643,15 @@ int snd_bebob_stream_start_duplex(struct snd_bebob 
*bebob, unsigned int rate)
dev_err(>unit->device,
"fail to ensure sampling rate: %d\n",
err);
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
-   goto end;
+   goto stop_rx_stream;
}
}
 
/* wait first callback */
if (!amdtp_stream_wait_callback(>rx_stream,
CALLBACK_TIMEOUT)) {
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
err = -ETIMEDOUT;
-   goto end;
+   goto stop_rx_stream;
}
}
 
@@ -666,9 +661,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
if (err < 0) {
dev_err(>unit->device,
"fail to run AMDTP slave stream:%d\n", err);
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
-   goto end;
+   goto stop_rx_stream;
}
 
/* wait first callback */
@@ -682,6 +675,12 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
}
 end:
return err;
+
+stop_rx_stream:
+   amdtp_stream_stop(>rx_stream);
+break_connections:
+   break_both_connections(bebob);
+   return err;
 }
 
 void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob)
-- 
2.14.1

[PATCH 1/3] ALSA: bebob: Use common error handling code in snd_bebob_stream_start_duplex()

[SF Markus Elfring]

From: Markus Elfring 
Date: Wed, 6 Sep 2017 11:40:53 +0200

Add jump targets so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 sound/firewire/bebob/bebob_stream.c | 21 ++---
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c 
b/sound/firewire/bebob/bebob_stream.c
index 4d3034a68bdf..bc9e42b6368e 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -629,8 +629,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
if (err < 0) {
dev_err(>unit->device,
"fail to run AMDTP master stream:%d\n", err);
-   break_both_connections(bebob);
-   goto end;
+   goto break_connections;
}
 
/*
@@ -644,19 +643,15 @@ int snd_bebob_stream_start_duplex(struct snd_bebob 
*bebob, unsigned int rate)
dev_err(>unit->device,
"fail to ensure sampling rate: %d\n",
err);
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
-   goto end;
+   goto stop_rx_stream;
}
}
 
/* wait first callback */
if (!amdtp_stream_wait_callback(>rx_stream,
CALLBACK_TIMEOUT)) {
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
err = -ETIMEDOUT;
-   goto end;
+   goto stop_rx_stream;
}
}
 
@@ -666,9 +661,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
if (err < 0) {
dev_err(>unit->device,
"fail to run AMDTP slave stream:%d\n", err);
-   amdtp_stream_stop(>rx_stream);
-   break_both_connections(bebob);
-   goto end;
+   goto stop_rx_stream;
}
 
/* wait first callback */
@@ -682,6 +675,12 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, 
unsigned int rate)
}
 end:
return err;
+
+stop_rx_stream:
+   amdtp_stream_stop(>rx_stream);
+break_connections:
+   break_both_connections(bebob);
+   return err;
 }
 
 void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob)
-- 
2.14.1