Messages in this thread | | | Subject | Re: [PATCH v2] cpupower:Fix resource leaks in sysfs_get_enabled() | From | Hao Zeng <> | Date | Mon, 17 Apr 2023 16:21:14 +0800 |
| |
Hi Shuah , Thanks very much for your review! I tried to understand what you said as best I could and provided V3. I look forward to your review again, it will help me a lot!
Best regards -- Hao
On Fri, 2023-04-14 at 10:41 -0600, Shuah Khan wrote: > On 4/13/23 21:08, Hao Zeng wrote: > > When the read return value is equal to 1, a file handle leak will > > occur > > > > Would like a bit more information on how the error paths are > redone, since memory leak isn't the only one that is fixed. > > > Signed-off-by: Hao Zeng <zenghao@kylinos.cn> > > Suggested-by: Shuah Khan <skhan@linuxfoundation.org> > > --- > > tools/power/cpupower/lib/powercap.c | 19 ++++++++++--------- > > 1 file changed, 10 insertions(+), 9 deletions(-) > > > > diff --git a/tools/power/cpupower/lib/powercap.c > > b/tools/power/cpupower/lib/powercap.c > > index 0ce29ee4c2e4..02ec5b0bff6b 100644 > > --- a/tools/power/cpupower/lib/powercap.c > > +++ b/tools/power/cpupower/lib/powercap.c > > @@ -40,7 +40,7 @@ static int sysfs_get_enabled(char *path, int > > *mode) > > { > > int fd; > > char yes_no; > > - > > + int ret = 0; > > *mode = 0; > > > > fd = open(path, O_RDONLY); > > @@ -48,17 +48,18 @@ static int sysfs_get_enabled(char *path, int > > *mode) > > return -1; > > > > if (read(fd, &yes_no, 1) != 1) { > > - close(fd); > > - return -1; > > + ret = -1; > > + goto err; > > } > > > > - if (yes_no == '1') { > > - *mode = 1; > > - return 0; > > - } else if (yes_no == '0') { > > - return 0; > > + if (yes_no != '1' || yes_no != '0') { > > + ret = -1; > > + goto err; > > } > > - return -1; > > + *mode = yes_no - '0'; > > I am not seeing much value in changing the above paths. > Leave them unchanged. > > > +err: > > + close(fd); > > + return ret; > > } > > > > int powercap_get_enabled(int *mode) > > thanks, > -- Shuah
| |