diff mbox series

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

Message ID 20210419192602.498815-6-vrzh@vrzh.net (mailing list archive)
State New, archived
Headers show
Series staging: media: atomisp: A series of cleanup | expand

Commit Message

Martiros Shakhzadyan April 19, 2021, 7:25 p.m. UTC 
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@vrzh.net>
---
 drivers/staging/media/atomisp/pci/sh_css.c | 72 +++++++++++-----------
 1 file changed, 35 insertions(+), 37 deletions(-)

Comments

Hans Verkuil April 20, 2021, 1:11 p.m. UTC | #1 
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@vrzh.net>
> ---
>  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
diff mbox series

Patch

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;
+	}
 
-				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;
+
+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) {
+			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