Re: [PATCH 5/8] staging: media: atomisp: Refactor ia_css_stream_load()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19/04/2021 21:25, Martiros Shakhzadyan wrote:
> Move the support check to the beginning of the function.
> Replace ENOTSUPP with a standard EOPNOTSUPP exit code.
> Use goto instead of nesting blocks in the stream search loop.
> Move variable assignment outside of the if statement.
> Remove an unnecessary check.
> 
> Signed-off-by: Martiros Shakhzadyan <vrzh@xxxxxxxx>
> ---
>  drivers/staging/media/atomisp/pci/sh_css.c | 72 +++++++++++-----------
>  1 file changed, 35 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/sh_css.c b/drivers/staging/media/atomisp/pci/sh_css.c
> index 87438b5948ba..71e8133abf04 100644
> --- a/drivers/staging/media/atomisp/pci/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/sh_css.c
> @@ -9616,48 +9616,46 @@ ia_css_stream_get_info(const struct ia_css_stream *stream,
>  int
>  ia_css_stream_load(struct ia_css_stream *stream)
>  {
> -	if (!IS_ISP2401) {
> -		int i;
> -		int err;
> +	int i, j, err;
>  
> -		assert(stream);
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
> -		for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
> -			if (my_css_save.stream_seeds[i].stream == stream) {
> -				int j;
> +	if (IS_ISP2401) {
> +		/* TODO remove function - DEPRECATED */
> +		(void)stream;
> +		return -EOPNOTSUPP;

Keep ENOTSUPP: ENOPNOTSUPP is for unsupported operands, ENOTSUPP is for
unsupported functionality, like this.

> +	}
>  
> -				for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
> -					if ((err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
> -								    &my_css_save.stream_seeds[i].pipes[j])) != 0) {
> -						if (j) {
> -							int k;
> +	assert(stream);
> +	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() enter,\n");
> +	for (i = 0; i < MAX_ACTIVE_STREAMS; i++)
> +		if (my_css_save.stream_seeds[i].stream == stream)
> +			goto found;
> +	goto done;

Yuck.

The way to do this is as follows:

	for (i = 0; i < MAX_ACTIVE_STREAMS; i++) {
		if (my_css_save.stream_seeds[i].stream != stream)
			continue;

		...

> +
> +found:
> +	for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++) {
> +		err = ia_css_pipe_create(&my_css_save.stream_seeds[i].pipe_config[j],
> +					 &my_css_save.stream_seeds[i].pipes[j]);
> +		if (err) {

Similar construct:

		if (!err)
			continue;

> +			int k;
>  
> -							for (k = 0; k < j; k++)
> -								ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
> -						}
> -						return err;
> -					}
> -				}
> -				err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
> -							my_css_save.stream_seeds[i].num_pipes,
> -							my_css_save.stream_seeds[i].pipes,
> -							&my_css_save.stream_seeds[i].stream);
> -				if (err) {
> -					ia_css_stream_destroy(stream);
> -					for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
> -						ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
> -					return err;
> -				}
> -				break;
> -			}
> +			for (k = 0; k < j; k++)
> +				ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
> +			return err;
>  		}
> -		ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
> -		return 0;
> -	} else {
> -		/* TODO remove function - DEPRECATED */
> -		(void)stream;
> -		return -ENOTSUPP;
>  	}
> +	err = ia_css_stream_create(&my_css_save.stream_seeds[i].stream_config,
> +				   my_css_save.stream_seeds[i].num_pipes,
> +				   my_css_save.stream_seeds[i].pipes,
> +				   &my_css_save.stream_seeds[i].stream);
> +	if (err) {
> +		ia_css_stream_destroy(stream);
> +		for (j = 0; j < my_css_save.stream_seeds[i].num_pipes; j++)
> +			ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
> +		return err;
> +	}
> +done:
> +	ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE,	"ia_css_stream_load() exit,\n");
> +	return 0;
>  }
>  
>  int
> 

Regards,

	Hans



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux