Re: [PATCH] staging: greybus: loopback_test: Change array index from loop bound to loop index

[Johan Hovold <johan@kernel.org>]

[+CC: greybus and staging lists]

On Sat, Feb 18, 2017 at 01:47:38PM +0530, sayli karnik wrote:
> Change array index from the loop bound variable to loop index.
> The open_poll_files() functions attempts to open poll files of devices
> numbered from 0 to device_count. If the open() function inside it is
> unsuccessful for any intermediate device, all files with fds of devices from 0
> upto that device must be closed and open_poll_files() should return -1.
> The current code only closes the poll file with the most recent fd allocated,
> and in most cases tries to close the same file multiple times.

Nice catch!

You forgot to CC the relevant mailings lists however (use
scripts/get_maintainer.pl).

Also your patch summary is a bit too detailed, something like

	staging: greybus: loopback_test: fix open error path

would be better.

Care to resend as a v2 with a shorter summary and lists on CC?

> Detected by coccinelle:
> 
> @@
> expression arr,ex1,ex2;
> @@
> 
> for(ex1 = 0; ex1 < ex2; ex1++) { <...
>   arr[
> - ex2
> + ex1
>   ]
>   ...> }
> 
> Signed-off-by: sayli karnik <karniksayli1995@gmail.com>
> ---
>  drivers/staging/greybus/tools/loopback_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c
> index 18d7a3d..2ee9a22 100644
> --- a/drivers/staging/greybus/tools/loopback_test.c
> +++ b/drivers/staging/greybus/tools/loopback_test.c
> @@ -674,7 +674,7 @@ static int open_poll_files(struct loopback_test *t)
>  
>  err:
>  	for (i = 0; i < fds_idx; i++)
> -		close(t->fds[fds_idx].fd);
> +		close(t->fds[i].fd);
>  
>  	return -1;
>  }

Thanks,
Johan