Re: [Powertop] [PATCH] powertop: fix various resource leaks

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&gt; 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&gt; >> 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&gt; >> --- >> 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