Messages in this thread | | | Subject | Re: [PATCH v3] cpupower:Fix resource leaks in sysfs_get_enabled() | From | Hao Zeng <> | Date | Tue, 18 Apr 2023 10:21:09 +0800 |
| |
Dear Shuah:
Thank you again for your patience and guidance. This has benefited me a lot. I will apply what I have learned this time to my work and kernel contribution
Best regards -- Hao
On Mon, 2023-04-17 at 15:09 -0600, Shuah Khan wrote: > On 4/17/23 01:56, Hao Zeng wrote: > > The sysfs_get_enabled() opened file processor not closed, > > may cause a file handle leak. > > Putting error handling and resource cleanup code together > > makes the code easy to maintain and read. > > Removed the unnecessary else if branch from the original > > function, as it should return an error in cases other than '0'. > > > > Signed-off-by: Hao Zeng <zenghao@kylinos.cn> > > Suggested-by: Shuah Khan <skhan@linuxfoundation.org> > > --- > > tools/power/cpupower/lib/powercap.c | 22 ++++++++++++++-------- > > 1 file changed, 14 insertions(+), 8 deletions(-) > > > > diff --git a/tools/power/cpupower/lib/powercap.c > > b/tools/power/cpupower/lib/powercap.c > > index 0ce29ee4c2e4..f0334a5f1acf 100644 > > --- a/tools/power/cpupower/lib/powercap.c > > +++ b/tools/power/cpupower/lib/powercap.c > > @@ -40,25 +40,31 @@ static int sysfs_get_enabled(char *path, int > > *mode) > > { > > int fd; > > char yes_no; > > + int ret = 0; > > > > *mode = 0; > > > > fd = open(path, O_RDONLY); > > - if (fd == -1) > > - return -1; > > + if (fd == -1) { > > + ret = -1; > > + goto out; > > + } > > > > if (read(fd, &yes_no, 1) != 1) { > > - close(fd); > > - return -1; > > + ret = -1; > > + goto out_close; > > } > > > > if (yes_no == '1') { > > *mode = 1; > > - return 0; > > You can just add goto out_close here > > > - } else if (yes_no == '0') { > > - return 0; > > Keep == '0' check and add goto out_close here > > + } else if (yes_no != '0') { > > This can be just else with the above change. > > > + ret = -1; > > + goto out_close; > > } > > - return -1; > > +out_close: > > + close(fd); > > +out: > > + return ret; > > } > > > > int powercap_get_enabled(int *mode) > > thanks, > -- Shuah
| |