* Re: [Powertop] [PATCH POWERTOP] Fix various resource leaks
@ 2014-07-02 10:53 Amit Kucheria
0 siblings, 0 replies; 8+ messages in thread
From: Amit Kucheria @ 2014-07-02 10:53 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 4731 bytes --]
Mohammad,
We've got a couple of patches that haven't been upstreamed yet. We'll
get to it. Sorry I didn't notice that your patch was to that code.
Regards,
Amit
On Wed, Jul 2, 2014 at 3:41 PM, Mohammad Merajul Islam Molla
<meraj.enigma(a)gmail.com> wrote:
> I cloned powertop repo from -
> git://git.linaro.org/power/powertop-2.0.git. When I build and run, I
> see an additional tab "Device Freq stats" and powertop version shows
> 2.4.
>
> I got the copy from github (pointed to by Sergey) now. When I build
> and run, I don't see any "Device Freq stats" tab and powertop version
> shows 2.6.1.
>
>
>
> On Tue, Jul 1, 2014 at 6:47 PM, Sergey Senozhatsky
> <sergey.senozhatsky(a)gmail.com> wrote:
>> On (07/01/14 17:54), Amit Kucheria wrote:
>>> Date: Tue, 1 Jul 2014 17:54:15 +0530
>>> From: Amit Kucheria <amit.kucheria(a)linaro.org>
>>> To: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>,
>>> powertop(a)lists.01.org
>>> Cc: Lists linaro-dev <linaro-dev(a)lists.linaro.org>
>>> Subject: Re: [Powertop] [PATCH POWERTOP] Fix various resource leaks
>>>
>>> Mohammad,
>>>
>>> This fix should go upstream. cc'ing the powertop list.
>>>
>>> Regards,
>>> Amit
>>>
>>> On Thu, Jun 26, 2014 at 12:42 PM, Mohammad Merajul Islam Molla
>>> <meraj.enigma(a)gmail.com> wrote:
>>> > Fixes some resource leaks detected by valgrind and coverity scan.
>>> >
>>> >
>>> > diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
>>> > index ac06460..7f704b6 100644
>>> > --- a/src/devices/ahci.cpp
>>> > +++ b/src/devices/ahci.cpp
>>> > @@ -64,8 +64,10 @@ static string disk_name(char *path, char *target,
>>> > char *shortname)
>>> > sprintf(line, "%s/%s/model", pathname, dirent->d_name);
>>> > file = fopen(line, "r");
>>> > if (file) {
>>> > - if (fgets(line, 4096, file) == NULL)
>>> > + if (fgets(line, 4096, file) == NULL) {
>>> > + fclose(file);
>>> > break;
>>> > + }
>>> > fclose(file);
>>> > c = strchr(line, '\n');
>>> > if (c)
>>> > diff --git a/src/devices/devfreq.cpp b/src/devices/devfreq.cpp
>>> > index e16951c..23c4b0c 100644
>>> > --- a/src/devices/devfreq.cpp
>>> > +++ b/src/devices/devfreq.cpp
>>> > @@ -238,6 +238,7 @@ void create_all_devfreq_devices(void)
>>> >
>>> > callback fn = &devfreq_dev_callback;
>>> > process_directory(p.c_str(), fn);
>>> > + closedir(dir);
>>
>> I don't see this file at
>> https://github.com/fenrus75/powertop/tree/master/src/devices
>>
>> afair, process_directory() closes dir.
>>
>> -ss
>>
>>> > }
>>> >
>>> > void initialize_devfreq(void)
>>> > diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
>>> > index b0e982b..cf1ae11 100644
>>> > --- a/src/perf/perf_bundle.cpp
>>> > +++ b/src/perf/perf_bundle.cpp
>>> > @@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
>>> >
>>> > buf = read_file(file);
>>> > free(file);
>>> > - if (!buf)
>>> > + if (!buf) {
>>> > + free(name);
>>> > return;
>>> > + }
>>> >
>>> > pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
>>> > free(name);
>>> > diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
>>> > index e0bdf12..5100a8a 100644
>>> > --- a/src/tuning/bluetooth.cpp
>>> > +++ b/src/tuning/bluetooth.cpp
>>> > @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
>>> > if (file) {
>>> > char line[2048];
>>> > /* first line is standard header */
>>> > - if (fgets(line, 2047, file) == NULL)
>>> > + if (fgets(line, 2047, file) == NULL) {
>>> > + pclose(file);
>>> > goto out;
>>> > + }
>>> > memset(line, 0, 2048);
>>> > if (fgets(line, 2047, file) == NULL) {
>>> > result = last_check_result = TUNE_GOOD;
>>> >
>>> >
>>> >
>>> > --
>>> > Thanks,
>>> > -Meraj
>>> >
>>> > _______________________________________________
>>> > linaro-dev mailing list
>>> > linaro-dev(a)lists.linaro.org
>>> > http://lists.linaro.org/mailman/listinfo/linaro-dev
>>> >
>>> _______________________________________________
>>> PowerTop mailing list
>>> PowerTop(a)lists.01.org
>>> https://lists.01.org/mailman/listinfo/powertop
>>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Powertop] [PATCH] powertop: fix various resource leaks
@ 2014-08-05 15:24 Sergey Senozhatsky
0 siblings, 0 replies; 8+ messages in thread
From: Sergey Senozhatsky @ 2014-08-05 15:24 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 5018 bytes --]
On (08/05/14 20:47), Mohammad Merajul Islam Molla wrote:
> Hi Sergey,
>
> Thanks for your review.
>
> For this portion of changes, is the below diff ok? I will resend the
> full patch if it is.
>
looks good.
-ss
> diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
> index 2c1e283..9be327e 100644
> --- a/src/tuning/bluetooth.cpp
> +++ b/src/tuning/bluetooth.cpp
> @@ -108,7 +108,7 @@ static int last_check_result;
> int bt_tunable::good_bad(void)
> {
> struct hci_dev_info devinfo;
> - FILE *file;
> + FILE *file = 0;
> int fd;
> int thisbytes = 0;
> int ret;
> @@ -144,18 +144,14 @@ int bt_tunable::good_bad(void)
> if (file) {
> char line[2048];
> /* first line is standard header */
> - if (fgets(line, 2047, file) == NULL) {
> - pclose(file);
> + if (fgets(line, 2047, file) == NULL)
> goto out;
> - }
> memset(line, 0, 2048);
> if (fgets(line, 2047, file) == NULL) {
> result = last_check_result = TUNE_GOOD;
> - pclose(file);
> goto out;
> }
>
> - pclose(file);
> if (strlen(line) > 0) {
> result = last_check_result = TUNE_GOOD;
> goto out;
> @@ -168,6 +164,8 @@ int bt_tunable::good_bad(void)
>
> out:
> previous_bytes = thisbytes;
> + if (file)
> + pclose(file);
> close(fd);
> return result;
> }
>
>
> --
> Thanks,
> -Meraj
>
>
>
> On Tue, Aug 5, 2014 at 6:44 PM, Sergey Senozhatsky
> <sergey.senozhatsky(a)gmail.com> wrote:
> > On (08/04/14 19:50), Mohammad Merajul Islam Molla wrote:
> >> Date: Mon, 4 Aug 2014 19:50:57 +0600
> >> From: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>
> >> To: powertop(a)lists.01.org
> >> Subject: [Powertop] [PATCH] powertop: fix various resource leaks
> >> X-Mailer: git-send-email 1.9.1
> >>
> >> fixes some resource leaks detected by valgrind and coverity scan.
> >>
> >> Signed-off-by: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>
> >> ---
> >> src/devices/ahci.cpp | 4 +++-
> >> src/perf/perf_bundle.cpp | 4 +++-
> >> src/tuning/bluetooth.cpp | 4 +++-
> >> 3 files changed, 9 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
> >> index 3b4627a..a4d1345 100644
> >> --- a/src/devices/ahci.cpp
> >> +++ b/src/devices/ahci.cpp
> >> @@ -67,8 +67,10 @@ static string disk_name(char *path, char *target, char *shortname)
> >> sprintf(line, "%s/%s/model", pathname, dirent->d_name);
> >> file = fopen(line, "r");
> >> if (file) {
> >> - if (fgets(line, 4096, file) == NULL)
> >> + if (fgets(line, 4096, file) == NULL) {
> >> + fclose(file);
> >> break;
> >> + }
> >> fclose(file);
> >> c = strchr(line, '\n');
> >> if (c)
> >> diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
> >> index b0e982b..cf1ae11 100644
> >> --- a/src/perf/perf_bundle.cpp
> >> +++ b/src/perf/perf_bundle.cpp
> >> @@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
> >>
> >> buf = read_file(file);
> >> free(file);
> >> - if (!buf)
> >> + if (!buf) {
> >> + free(name);
> >> return;
> >> + }
> >>
> >> pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
> >> free(name);
> >> diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
> >> index 92f5835..2c1e283 100644
> >> --- a/src/tuning/bluetooth.cpp
> >> +++ b/src/tuning/bluetooth.cpp
> >> @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
> >> if (file) {
> >> char line[2048];
> >> /* first line is standard header */
> >> - if (fgets(line, 2047, file) == NULL)
> >> + if (fgets(line, 2047, file) == NULL) {
> >> + pclose(file);
> >> goto out;
> >> + }
> >> memset(line, 0, 2048);
> >> if (fgets(line, 2047, file) == NULL) {
> >> result = last_check_result = TUNE_GOOD;
> >>
> >
> > how about moving pclose(file) to `out:' label scope? 3 out of 11 lines
> > are doing same work here: cleaning up resource before jump.
> >
> > -ss
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Powertop] [PATCH] powertop: fix various resource leaks
@ 2014-08-05 14:47 Mohammad Merajul Islam Molla
0 siblings, 0 replies; 8+ messages in thread
From: Mohammad Merajul Islam Molla @ 2014-08-05 14:47 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 4673 bytes --]
Hi Sergey,
Thanks for your review.
For this portion of changes, is the below diff ok? I will resend the
full patch if it is.
diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
index 2c1e283..9be327e 100644
--- a/src/tuning/bluetooth.cpp
+++ b/src/tuning/bluetooth.cpp
@@ -108,7 +108,7 @@ static int last_check_result;
int bt_tunable::good_bad(void)
{
struct hci_dev_info devinfo;
- FILE *file;
+ FILE *file = 0;
int fd;
int thisbytes = 0;
int ret;
@@ -144,18 +144,14 @@ int bt_tunable::good_bad(void)
if (file) {
char line[2048];
/* first line is standard header */
- if (fgets(line, 2047, file) == NULL) {
- pclose(file);
+ if (fgets(line, 2047, file) == NULL)
goto out;
- }
memset(line, 0, 2048);
if (fgets(line, 2047, file) == NULL) {
result = last_check_result = TUNE_GOOD;
- pclose(file);
goto out;
}
- pclose(file);
if (strlen(line) > 0) {
result = last_check_result = TUNE_GOOD;
goto out;
@@ -168,6 +164,8 @@ int bt_tunable::good_bad(void)
out:
previous_bytes = thisbytes;
+ if (file)
+ pclose(file);
close(fd);
return result;
}
--
Thanks,
-Meraj
On Tue, Aug 5, 2014 at 6:44 PM, Sergey Senozhatsky
<sergey.senozhatsky(a)gmail.com> wrote:
> On (08/04/14 19:50), Mohammad Merajul Islam Molla wrote:
>> Date: Mon, 4 Aug 2014 19:50:57 +0600
>> From: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>
>> To: powertop(a)lists.01.org
>> Subject: [Powertop] [PATCH] powertop: fix various resource leaks
>> X-Mailer: git-send-email 1.9.1
>>
>> fixes some resource leaks detected by valgrind and coverity scan.
>>
>> Signed-off-by: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>
>> ---
>> src/devices/ahci.cpp | 4 +++-
>> src/perf/perf_bundle.cpp | 4 +++-
>> src/tuning/bluetooth.cpp | 4 +++-
>> 3 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
>> index 3b4627a..a4d1345 100644
>> --- a/src/devices/ahci.cpp
>> +++ b/src/devices/ahci.cpp
>> @@ -67,8 +67,10 @@ static string disk_name(char *path, char *target, char *shortname)
>> sprintf(line, "%s/%s/model", pathname, dirent->d_name);
>> file = fopen(line, "r");
>> if (file) {
>> - if (fgets(line, 4096, file) == NULL)
>> + if (fgets(line, 4096, file) == NULL) {
>> + fclose(file);
>> break;
>> + }
>> fclose(file);
>> c = strchr(line, '\n');
>> if (c)
>> diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
>> index b0e982b..cf1ae11 100644
>> --- a/src/perf/perf_bundle.cpp
>> +++ b/src/perf/perf_bundle.cpp
>> @@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
>>
>> buf = read_file(file);
>> free(file);
>> - if (!buf)
>> + if (!buf) {
>> + free(name);
>> return;
>> + }
>>
>> pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
>> free(name);
>> diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
>> index 92f5835..2c1e283 100644
>> --- a/src/tuning/bluetooth.cpp
>> +++ b/src/tuning/bluetooth.cpp
>> @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
>> if (file) {
>> char line[2048];
>> /* first line is standard header */
>> - if (fgets(line, 2047, file) == NULL)
>> + if (fgets(line, 2047, file) == NULL) {
>> + pclose(file);
>> goto out;
>> + }
>> memset(line, 0, 2048);
>> if (fgets(line, 2047, file) == NULL) {
>> result = last_check_result = TUNE_GOOD;
>>
>
> how about moving pclose(file) to `out:' label scope? 3 out of 11 lines
> are doing same work here: cleaning up resource before jump.
>
> -ss
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Powertop] [PATCH] powertop: fix various resource leaks
@ 2014-08-05 12:44 Sergey Senozhatsky
0 siblings, 0 replies; 8+ messages in thread
From: Sergey Senozhatsky @ 2014-08-05 12:44 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 2359 bytes --]
On (08/04/14 19:50), Mohammad Merajul Islam Molla wrote:
> Date: Mon, 4 Aug 2014 19:50:57 +0600
> From: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>
> To: powertop(a)lists.01.org
> Subject: [Powertop] [PATCH] powertop: fix various resource leaks
> X-Mailer: git-send-email 1.9.1
>
> fixes some resource leaks detected by valgrind and coverity scan.
>
> Signed-off-by: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>
> ---
> src/devices/ahci.cpp | 4 +++-
> src/perf/perf_bundle.cpp | 4 +++-
> src/tuning/bluetooth.cpp | 4 +++-
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
> index 3b4627a..a4d1345 100644
> --- a/src/devices/ahci.cpp
> +++ b/src/devices/ahci.cpp
> @@ -67,8 +67,10 @@ static string disk_name(char *path, char *target, char *shortname)
> sprintf(line, "%s/%s/model", pathname, dirent->d_name);
> file = fopen(line, "r");
> if (file) {
> - if (fgets(line, 4096, file) == NULL)
> + if (fgets(line, 4096, file) == NULL) {
> + fclose(file);
> break;
> + }
> fclose(file);
> c = strchr(line, '\n');
> if (c)
> diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
> index b0e982b..cf1ae11 100644
> --- a/src/perf/perf_bundle.cpp
> +++ b/src/perf/perf_bundle.cpp
> @@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
>
> buf = read_file(file);
> free(file);
> - if (!buf)
> + if (!buf) {
> + free(name);
> return;
> + }
>
> pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
> free(name);
> diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
> index 92f5835..2c1e283 100644
> --- a/src/tuning/bluetooth.cpp
> +++ b/src/tuning/bluetooth.cpp
> @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
> if (file) {
> char line[2048];
> /* first line is standard header */
> - if (fgets(line, 2047, file) == NULL)
> + if (fgets(line, 2047, file) == NULL) {
> + pclose(file);
> goto out;
> + }
> memset(line, 0, 2048);
> if (fgets(line, 2047, file) == NULL) {
> result = last_check_result = TUNE_GOOD;
>
how about moving pclose(file) to `out:' label scope? 3 out of 11 lines
are doing same work here: cleaning up resource before jump.
-ss
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Powertop] [PATCH] powertop: fix various resource leaks
@ 2014-08-04 13:50 Mohammad Merajul Islam Molla
0 siblings, 0 replies; 8+ messages in thread
From: Mohammad Merajul Islam Molla @ 2014-08-04 13:50 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]
fixes some resource leaks detected by valgrind and coverity scan.
Signed-off-by: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>
---
src/devices/ahci.cpp | 4 +++-
src/perf/perf_bundle.cpp | 4 +++-
src/tuning/bluetooth.cpp | 4 +++-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
index 3b4627a..a4d1345 100644
--- a/src/devices/ahci.cpp
+++ b/src/devices/ahci.cpp
@@ -67,8 +67,10 @@ static string disk_name(char *path, char *target, char *shortname)
sprintf(line, "%s/%s/model", pathname, dirent->d_name);
file = fopen(line, "r");
if (file) {
- if (fgets(line, 4096, file) == NULL)
+ if (fgets(line, 4096, file) == NULL) {
+ fclose(file);
break;
+ }
fclose(file);
c = strchr(line, '\n');
if (c)
diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
index b0e982b..cf1ae11 100644
--- a/src/perf/perf_bundle.cpp
+++ b/src/perf/perf_bundle.cpp
@@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
buf = read_file(file);
free(file);
- if (!buf)
+ if (!buf) {
+ free(name);
return;
+ }
pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
free(name);
diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
index 92f5835..2c1e283 100644
--- a/src/tuning/bluetooth.cpp
+++ b/src/tuning/bluetooth.cpp
@@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
if (file) {
char line[2048];
/* first line is standard header */
- if (fgets(line, 2047, file) == NULL)
+ if (fgets(line, 2047, file) == NULL) {
+ pclose(file);
goto out;
+ }
memset(line, 0, 2048);
if (fgets(line, 2047, file) == NULL) {
result = last_check_result = TUNE_GOOD;
--
1.9.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Powertop] [PATCH POWERTOP] Fix various resource leaks
@ 2014-07-28 18:12 Alexandra Yates
0 siblings, 0 replies; 8+ messages in thread
From: Alexandra Yates @ 2014-07-28 18:12 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 3563 bytes --]
Hi Mohammad and Amit,
> Mohammad,
>
> This fix should go upstream. cc'ing the powertop list.
>
> Regards,
> Amit
>
> On Thu, Jun 26, 2014 at 12:42 PM, Mohammad Merajul Islam Molla
> <meraj.enigma(a)gmail.com> wrote:
>> Fixes some resource leaks detected by valgrind and coverity scan.
>>
>>
>> diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
>> index ac06460..7f704b6 100644
>> --- a/src/devices/ahci.cpp
>> +++ b/src/devices/ahci.cpp
>> @@ -64,8 +64,10 @@ static string disk_name(char *path, char *target,
>> char *shortname)
>> sprintf(line, "%s/%s/model", pathname, dirent->d_name);
>> file = fopen(line, "r");
>> if (file) {
>> - if (fgets(line, 4096, file) == NULL)
>> + if (fgets(line, 4096, file) == NULL) {
>> + fclose(file);
>> break;
>> + }
>> fclose(file);
>> c = strchr(line, '\n');
>> if (c)
>> diff --git a/src/devices/devfreq.cpp b/src/devices/devfreq.cpp
>> index e16951c..23c4b0c 100644
>> --- a/src/devices/devfreq.cpp
>> +++ b/src/devices/devfreq.cpp
>> @@ -238,6 +238,7 @@ void create_all_devfreq_devices(void)
>>
>> callback fn = &devfreq_dev_callback;
>> process_directory(p.c_str(), fn);
>> + closedir(dir);
>> }
>>
>> void initialize_devfreq(void)
>> diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
>> index b0e982b..cf1ae11 100644
>> --- a/src/perf/perf_bundle.cpp
>> +++ b/src/perf/perf_bundle.cpp
>> @@ -142,8 +142,10 @@ static void parse_event_format(const char
>> *event_name)
>>
>> buf = read_file(file);
>> free(file);
>> - if (!buf)
>> + if (!buf) {
>> + free(name);
>> return;
>> + }
>>
>> pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
>> free(name);
>> diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
>> index e0bdf12..5100a8a 100644
>> --- a/src/tuning/bluetooth.cpp
>> +++ b/src/tuning/bluetooth.cpp
>> @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
>> if (file) {
>> char line[2048];
>> /* first line is standard header */
>> - if (fgets(line, 2047, file) == NULL)
>> + if (fgets(line, 2047, file) == NULL) {
>> + pclose(file);
>> goto out;
>> + }
>> memset(line, 0, 2048);
>> if (fgets(line, 2047, file) == NULL) {
>> result = last_check_result = TUNE_GOOD;
>>
>>
>>
>> --
>> Thanks,
>> -Meraj
>>
>> _______________________________________________
>> linaro-dev mailing list
>> linaro-dev(a)lists.linaro.org
>> http://lists.linaro.org/mailman/listinfo/linaro-dev
>>
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
>
Please send you patch against the tip of the tree. I'm having
difficulties adding it:
Here is the error message:
Patch is empty. Was it split wrong?
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Thank you,
Alexandra.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Powertop] [PATCH POWERTOP] Fix various resource leaks
@ 2014-07-01 12:47 Sergey Senozhatsky
0 siblings, 0 replies; 8+ messages in thread
From: Sergey Senozhatsky @ 2014-07-01 12:47 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 3789 bytes --]
On (07/01/14 17:54), Amit Kucheria wrote:
> Date: Tue, 1 Jul 2014 17:54:15 +0530
> From: Amit Kucheria <amit.kucheria(a)linaro.org>
> To: Mohammad Merajul Islam Molla <meraj.enigma(a)gmail.com>,
> powertop(a)lists.01.org
> Cc: Lists linaro-dev <linaro-dev(a)lists.linaro.org>
> Subject: Re: [Powertop] [PATCH POWERTOP] Fix various resource leaks
>
> Mohammad,
>
> This fix should go upstream. cc'ing the powertop list.
>
> Regards,
> Amit
>
> On Thu, Jun 26, 2014 at 12:42 PM, Mohammad Merajul Islam Molla
> <meraj.enigma(a)gmail.com> wrote:
> > Fixes some resource leaks detected by valgrind and coverity scan.
> >
> >
> > diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
> > index ac06460..7f704b6 100644
> > --- a/src/devices/ahci.cpp
> > +++ b/src/devices/ahci.cpp
> > @@ -64,8 +64,10 @@ static string disk_name(char *path, char *target,
> > char *shortname)
> > sprintf(line, "%s/%s/model", pathname, dirent->d_name);
> > file = fopen(line, "r");
> > if (file) {
> > - if (fgets(line, 4096, file) == NULL)
> > + if (fgets(line, 4096, file) == NULL) {
> > + fclose(file);
> > break;
> > + }
> > fclose(file);
> > c = strchr(line, '\n');
> > if (c)
> > diff --git a/src/devices/devfreq.cpp b/src/devices/devfreq.cpp
> > index e16951c..23c4b0c 100644
> > --- a/src/devices/devfreq.cpp
> > +++ b/src/devices/devfreq.cpp
> > @@ -238,6 +238,7 @@ void create_all_devfreq_devices(void)
> >
> > callback fn = &devfreq_dev_callback;
> > process_directory(p.c_str(), fn);
> > + closedir(dir);
I don't see this file at
https://github.com/fenrus75/powertop/tree/master/src/devices
afair, process_directory() closes dir.
-ss
> > }
> >
> > void initialize_devfreq(void)
> > diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
> > index b0e982b..cf1ae11 100644
> > --- a/src/perf/perf_bundle.cpp
> > +++ b/src/perf/perf_bundle.cpp
> > @@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
> >
> > buf = read_file(file);
> > free(file);
> > - if (!buf)
> > + if (!buf) {
> > + free(name);
> > return;
> > + }
> >
> > pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
> > free(name);
> > diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
> > index e0bdf12..5100a8a 100644
> > --- a/src/tuning/bluetooth.cpp
> > +++ b/src/tuning/bluetooth.cpp
> > @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
> > if (file) {
> > char line[2048];
> > /* first line is standard header */
> > - if (fgets(line, 2047, file) == NULL)
> > + if (fgets(line, 2047, file) == NULL) {
> > + pclose(file);
> > goto out;
> > + }
> > memset(line, 0, 2048);
> > if (fgets(line, 2047, file) == NULL) {
> > result = last_check_result = TUNE_GOOD;
> >
> >
> >
> > --
> > Thanks,
> > -Meraj
> >
> > _______________________________________________
> > linaro-dev mailing list
> > linaro-dev(a)lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/linaro-dev
> >
> _______________________________________________
> PowerTop mailing list
> PowerTop(a)lists.01.org
> https://lists.01.org/mailman/listinfo/powertop
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Powertop] [PATCH POWERTOP] Fix various resource leaks
@ 2014-07-01 12:24 Amit Kucheria
0 siblings, 0 replies; 8+ messages in thread
From: Amit Kucheria @ 2014-07-01 12:24 UTC (permalink / raw)
To: powertop
[-- Attachment #1: Type: text/plain, Size: 2962 bytes --]
Mohammad,
This fix should go upstream. cc'ing the powertop list.
Regards,
Amit
On Thu, Jun 26, 2014 at 12:42 PM, Mohammad Merajul Islam Molla
<meraj.enigma(a)gmail.com> wrote:
> Fixes some resource leaks detected by valgrind and coverity scan.
>
>
> diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
> index ac06460..7f704b6 100644
> --- a/src/devices/ahci.cpp
> +++ b/src/devices/ahci.cpp
> @@ -64,8 +64,10 @@ static string disk_name(char *path, char *target,
> char *shortname)
> sprintf(line, "%s/%s/model", pathname, dirent->d_name);
> file = fopen(line, "r");
> if (file) {
> - if (fgets(line, 4096, file) == NULL)
> + if (fgets(line, 4096, file) == NULL) {
> + fclose(file);
> break;
> + }
> fclose(file);
> c = strchr(line, '\n');
> if (c)
> diff --git a/src/devices/devfreq.cpp b/src/devices/devfreq.cpp
> index e16951c..23c4b0c 100644
> --- a/src/devices/devfreq.cpp
> +++ b/src/devices/devfreq.cpp
> @@ -238,6 +238,7 @@ void create_all_devfreq_devices(void)
>
> callback fn = &devfreq_dev_callback;
> process_directory(p.c_str(), fn);
> + closedir(dir);
> }
>
> void initialize_devfreq(void)
> diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
> index b0e982b..cf1ae11 100644
> --- a/src/perf/perf_bundle.cpp
> +++ b/src/perf/perf_bundle.cpp
> @@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
>
> buf = read_file(file);
> free(file);
> - if (!buf)
> + if (!buf) {
> + free(name);
> return;
> + }
>
> pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
> free(name);
> diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
> index e0bdf12..5100a8a 100644
> --- a/src/tuning/bluetooth.cpp
> +++ b/src/tuning/bluetooth.cpp
> @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
> if (file) {
> char line[2048];
> /* first line is standard header */
> - if (fgets(line, 2047, file) == NULL)
> + if (fgets(line, 2047, file) == NULL) {
> + pclose(file);
> goto out;
> + }
> memset(line, 0, 2048);
> if (fgets(line, 2047, file) == NULL) {
> result = last_check_result = TUNE_GOOD;
>
>
>
> --
> Thanks,
> -Meraj
>
> _______________________________________________
> linaro-dev mailing list
> linaro-dev(a)lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-08-05 15:24 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-02 10:53 [Powertop] [PATCH POWERTOP] Fix various resource leaks Amit Kucheria
-- strict thread matches above, loose matches on Subject: below --
2014-08-05 15:24 [Powertop] [PATCH] powertop: fix " Sergey Senozhatsky
2014-08-05 14:47 Mohammad Merajul Islam Molla
2014-08-05 12:44 Sergey Senozhatsky
2014-08-04 13:50 Mohammad Merajul Islam Molla
2014-07-28 18:12 [Powertop] [PATCH POWERTOP] Fix " Alexandra Yates
2014-07-01 12:47 Sergey Senozhatsky
2014-07-01 12:24 Amit Kucheria
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.