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

On Mon, Feb 20, 2017 at 5:30 PM, Johan Hovold johan@kernel.org wrote:

[+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).

Yes I'll do that!

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?

Yes sure I'm sending a v2.

thanks, sayli

...

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