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

Groups > linux.kernel > #1738106 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/3] ALSA: bebob: Use common error handling code in  snd_bebob_stream_start_duplex() Takashi Sakamoto <o-takashi@sakamocchi.jp> - 2017-09-24 06:00 +0200
    Re: ALSA: bebob: Use common error handling code in  snd_bebob_stream_start_duplex() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-09-24 09:10 +0200

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

Hi,

On Sep 6 2017 19:22, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> 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 <elfring@users.sourceforge.net>
> ---
>   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(&bebob->unit->device,
>   				"fail to run AMDTP slave stream:%d\n", err);
> -			amdtp_stream_stop(&bebob->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(&bebob->tx_stream)) {
660     err = start_stream(bebob, &bebob->tx_stream, rate);
661     if (err < 0) {
662         dev_err(&bebob->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(&bebob->tx_stream,
669                                     CALLBACK_TIMEOUT)) {
670         amdtp_stream_stop(&bebob->tx_stream);
671         amdtp_stream_stop(&bebob->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(&bebob->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

[toc] | [next] | [standalone]

#1738114 — Re: ALSA: bebob: Use common error handling code in snd_bebob_stream_start_duplex()

> 668     if (!amdtp_stream_wait_callback(&bebob->tx_stream,
> 669                                     CALLBACK_TIMEOUT)) {
> 670         amdtp_stream_stop(&bebob->tx_stream);
> 671         amdtp_stream_stop(&bebob->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.

How do you think about to adjust this function implementation after the other two
update steps from the patch series would be integrated?


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

Would you like to refer to any specific update suggestions for further clarification?

Regards,
Markus

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web