* [PATCH] staging: greybus: loopback_test: Change array index from loop bound to loop index
@ 2017-02-18 8:17 sayli karnik
2017-02-20 12:00 ` Johan Hovold
0 siblings, 1 reply; 3+ messages in thread
From: sayli karnik @ 2017-02-18 8:17 UTC (permalink / raw)
To: outreachy-kernel; +Cc: Johan Hovold, Alex Elder, Greg Kroah-Hartman
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.
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;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: greybus: loopback_test: Change array index from loop bound to loop index
2017-02-18 8:17 [PATCH] staging: greybus: loopback_test: Change array index from loop bound to loop index sayli karnik
@ 2017-02-20 12:00 ` Johan Hovold
2017-02-20 16:21 ` sayli karnik
0 siblings, 1 reply; 3+ messages in thread
From: Johan Hovold @ 2017-02-20 12:00 UTC (permalink / raw)
To: sayli karnik
Cc: outreachy-kernel, Johan Hovold, Alex Elder, Greg Kroah-Hartman,
greybus-dev, devel
[+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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: greybus: loopback_test: Change array index from loop bound to loop index
2017-02-20 12:00 ` Johan Hovold
@ 2017-02-20 16:21 ` sayli karnik
0 siblings, 0 replies; 3+ messages in thread
From: sayli karnik @ 2017-02-20 16:21 UTC (permalink / raw)
To: Johan Hovold
Cc: outreachy-kernel, Alex Elder, Greg Kroah-Hartman, greybus-dev, devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-02-20 16:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-18 8:17 [PATCH] staging: greybus: loopback_test: Change array index from loop bound to loop index sayli karnik
2017-02-20 12:00 ` Johan Hovold
2017-02-20 16:21 ` sayli karnik
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.