* [PATCH] staging: greybus: loop variable to loop index
@ 2017-02-21 16:59 Gargi Sharma
2017-02-21 17:31 ` [Outreachy kernel] " Julia Lawall
0 siblings, 1 reply; 5+ messages in thread
From: Gargi Sharma @ 2017-02-21 16:59 UTC (permalink / raw)
To: outreachy-kernel; +Cc: gregkh, johan, elder, Gargi Sharma
Change array index from the loop bound variable to loop index.
Once all the loopback devices have been found, free the pointer
array containing the dirent structures. The pointer to dirent
structures must be individually freed before freeing the pointer
array.
Coccinelle Script:
@@
expression arr,ex1,ex2;
@@
for(ex1 = 0; ex1 < ex2; ex1++) { <...
arr[
- ex2
+ ex1
]
...> }
Signed-off-by: Gargi Sharma <gs051095@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..1c01833 100644
--- a/drivers/staging/greybus/tools/loopback_test.c
+++ b/drivers/staging/greybus/tools/loopback_test.c
@@ -636,7 +636,7 @@ int find_loopback_devices(struct loopback_test *t)
ret = 0;
done:
for (i = 0; i < n; i++)
- free(namelist[n]);
+ free(namelist[i]);
free(namelist);
baddir:
return ret;
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Outreachy kernel] [PATCH] staging: greybus: loop variable to loop index 2017-02-21 16:59 [PATCH] staging: greybus: loop variable to loop index Gargi Sharma @ 2017-02-21 17:31 ` Julia Lawall 2017-02-21 18:19 ` Johan Hovold 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2017-02-21 17:31 UTC (permalink / raw) To: Gargi Sharma; +Cc: outreachy-kernel, gregkh, johan, elder On Tue, 21 Feb 2017, Gargi Sharma wrote: > Change array index from the loop bound variable to loop index. > Once all the loopback devices have been found, free the pointer > array containing the dirent structures. The pointer to dirent > structures must be individually freed before freeing the pointer > array. > > Coccinelle Script: > @@ > expression arr,ex1,ex2; > @@ > > for(ex1 = 0; ex1 < ex2; ex1++) { <... > arr[ > - ex2 > + ex1 > ] > ...> } > > Signed-off-by: Gargi Sharma <gs051095@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..1c01833 100644 > --- a/drivers/staging/greybus/tools/loopback_test.c > +++ b/drivers/staging/greybus/tools/loopback_test.c > @@ -636,7 +636,7 @@ int find_loopback_devices(struct loopback_test *t) > ret = 0; > done: > for (i = 0; i < n; i++) > - free(namelist[n]); > + free(namelist[i]); I guess this is a test, so no one ever tried running it :) julia > free(namelist); > baddir: > return ret; > -- > 2.7.4 > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1487696353-11316-1-git-send-email-gs051095%40gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: greybus: loop variable to loop index 2017-02-21 17:31 ` [Outreachy kernel] " Julia Lawall @ 2017-02-21 18:19 ` Johan Hovold 2017-02-21 19:50 ` Julia Lawall 0 siblings, 1 reply; 5+ messages in thread From: Johan Hovold @ 2017-02-21 18:19 UTC (permalink / raw) To: Julia Lawall; +Cc: Gargi Sharma, outreachy-kernel, gregkh, johan, elder On Tue, Feb 21, 2017 at 06:31:28PM +0100, Julia Lawall wrote: > On Tue, 21 Feb 2017, Gargi Sharma wrote: > > > Change array index from the loop bound variable to loop index. > > Once all the loopback devices have been found, free the pointer > > array containing the dirent structures. The pointer to dirent > > structures must be individually freed before freeing the pointer > > array. > > > > Coccinelle Script: > > @@ > > expression arr,ex1,ex2; > > @@ > > > > for(ex1 = 0; ex1 < ex2; ex1++) { <... > > arr[ > > - ex2 > > + ex1 > > ] > > ...> } Another good catch, but you should change the patch summary to something more descriptive (and less detailed) like: staging: greybus: loopback_test: fix device-name leak and possibly also make it more clear from the commit message that we've been leaking these strings whenever there have been more than one loopback device. (The memory is returned once the test program exits, but still). Care to send a v2 with those changes? > > Signed-off-by: Gargi Sharma <gs051095@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..1c01833 100644 > > --- a/drivers/staging/greybus/tools/loopback_test.c > > +++ b/drivers/staging/greybus/tools/loopback_test.c > > @@ -636,7 +636,7 @@ int find_loopback_devices(struct loopback_test *t) > > ret = 0; > > done: > > for (i = 0; i < n; i++) > > - free(namelist[n]); > > + free(namelist[i]); > > I guess this is a test, so no one ever tried running it :) It is a test tool, but we've been using it quite a bit. Thanks, Johan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: greybus: loop variable to loop index 2017-02-21 18:19 ` Johan Hovold @ 2017-02-21 19:50 ` Julia Lawall 2017-02-21 19:54 ` Johan Hovold 0 siblings, 1 reply; 5+ messages in thread From: Julia Lawall @ 2017-02-21 19:50 UTC (permalink / raw) To: Johan Hovold; +Cc: Julia Lawall, Gargi Sharma, outreachy-kernel, gregkh, elder On Tue, 21 Feb 2017, Johan Hovold wrote: > On Tue, Feb 21, 2017 at 06:31:28PM +0100, Julia Lawall wrote: > > On Tue, 21 Feb 2017, Gargi Sharma wrote: > > > > > Change array index from the loop bound variable to loop index. > > > Once all the loopback devices have been found, free the pointer > > > array containing the dirent structures. The pointer to dirent > > > structures must be individually freed before freeing the pointer > > > array. > > > > > > Coccinelle Script: > > > @@ > > > expression arr,ex1,ex2; > > > @@ > > > > > > for(ex1 = 0; ex1 < ex2; ex1++) { <... > > > arr[ > > > - ex2 > > > + ex1 > > > ] > > > ...> } > > Another good catch, but you should change the patch summary to something > more descriptive (and less detailed) like: > > staging: greybus: loopback_test: fix device-name leak > > and possibly also make it more clear from the commit message that we've > been leaking these strings whenever there have been more than one > loopback device. (The memory is returned once the test program exits, > but still). > > Care to send a v2 with those changes? > > > > Signed-off-by: Gargi Sharma <gs051095@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..1c01833 100644 > > > --- a/drivers/staging/greybus/tools/loopback_test.c > > > +++ b/drivers/staging/greybus/tools/loopback_test.c > > > @@ -636,7 +636,7 @@ int find_loopback_devices(struct loopback_test *t) > > > ret = 0; > > > done: > > > for (i = 0; i < n; i++) > > > - free(namelist[n]); > > > + free(namelist[i]); > > > > I guess this is a test, so no one ever tried running it :) > > It is a test tool, but we've been using it quite a bit. I guess I don't know what n is. I would have expected that it would be an out of bounds array reference, at least sometimes. julia > > Thanks, > Johan > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170221181950.GD10245%40localhost. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: greybus: loop variable to loop index 2017-02-21 19:50 ` Julia Lawall @ 2017-02-21 19:54 ` Johan Hovold 0 siblings, 0 replies; 5+ messages in thread From: Johan Hovold @ 2017-02-21 19:54 UTC (permalink / raw) To: Julia Lawall; +Cc: Johan Hovold, Gargi Sharma, outreachy-kernel, gregkh, elder On Tue, Feb 21, 2017 at 08:50:33PM +0100, Julia Lawall wrote: > On Tue, 21 Feb 2017, Johan Hovold wrote: > > On Tue, Feb 21, 2017 at 06:31:28PM +0100, Julia Lawall wrote: > > > On Tue, 21 Feb 2017, Gargi Sharma wrote: > > > > > > > Change array index from the loop bound variable to loop index. > > > > Once all the loopback devices have been found, free the pointer > > > > array containing the dirent structures. The pointer to dirent > > > > structures must be individually freed before freeing the pointer > > > > array. > > > > > > > > Coccinelle Script: > > > > @@ > > > > expression arr,ex1,ex2; > > > > @@ > > > > > > > > for(ex1 = 0; ex1 < ex2; ex1++) { <... > > > > arr[ > > > > - ex2 > > > > + ex1 > > > > ] > > > > ...> } > > > > Another good catch, but you should change the patch summary to something > > more descriptive (and less detailed) like: > > > > staging: greybus: loopback_test: fix device-name leak > > > > and possibly also make it more clear from the commit message that we've > > been leaking these strings whenever there have been more than one > > loopback device. (The memory is returned once the test program exits, > > but still). > > > > Care to send a v2 with those changes? > > > > > > Signed-off-by: Gargi Sharma <gs051095@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..1c01833 100644 > > > > --- a/drivers/staging/greybus/tools/loopback_test.c > > > > +++ b/drivers/staging/greybus/tools/loopback_test.c > > > > @@ -636,7 +636,7 @@ int find_loopback_devices(struct loopback_test *t) > > > > ret = 0; > > > > done: > > > > for (i = 0; i < n; i++) > > > > - free(namelist[n]); > > > > + free(namelist[i]); > > > > > > I guess this is a test, so no one ever tried running it :) > > > > It is a test tool, but we've been using it quite a bit. > > I guess I don't know what n is. I would have expected that it would be an > out of bounds array reference, at least sometimes. No, you're absolutely right, I just noticed the potential illegal free here too. Perhaps namelist[n] has just happened to be NULL so far. Thanks, Johan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-02-21 19:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-21 16:59 [PATCH] staging: greybus: loop variable to loop index Gargi Sharma 2017-02-21 17:31 ` [Outreachy kernel] " Julia Lawall 2017-02-21 18:19 ` Johan Hovold 2017-02-21 19:50 ` Julia Lawall 2017-02-21 19:54 ` Johan Hovold
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.