| Message ID | 20230413044602.3221453-1-zenghao@kylinos.cn (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Shuah Khan |
| Headers | show |
| Series | cpupower:Fix resource leaks in sysfs_get_enabled() | expand |
On 4/12/23 22:46, Hao Zeng wrote: > The sysfs_get_enabled() opened file processor not closed, > may cause a file handle leak. > Please add information how you found this problem? > Signed-off-by: Hao Zeng <zenghao@kylinos.cn> > --- > tools/power/cpupower/lib/powercap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/power/cpupower/lib/powercap.c b/tools/power/cpupower/lib/powercap.c > index 0ce29ee4c2e4..a39ee1c89679 100644 > --- a/tools/power/cpupower/lib/powercap.c > +++ b/tools/power/cpupower/lib/powercap.c > @@ -51,7 +51,7 @@ static int sysfs_get_enabled(char *path, int *mode) > close(fd); > return -1; > }> - > + close(fd); The error path logic can be simplified with a goto to to handle the error path to close the file and return. > if (yes_no == '1') { > *mode = 1; > return 0; This path that returns 0 can be simplified as well to do return just once, after error path handling is done with a goto. Please send me v2 with the changes above with information on how you found the problem. thanks, -- Shuah thanks, -- Shuah
Dear Shuah Thank you for taking the time to reply to my email. On Thu, 2023-04-13 at 10:43 -0600, Shuah Khan wrote: > On 4/12/23 22:46, Hao Zeng wrote: > > The sysfs_get_enabled() opened file processor not closed, > > may cause a file handle leak. > > > > Please add information how you found this problem? > > > Signed-off-by: Hao Zeng <zenghao@kylinos.cn> > > --- > > tools/power/cpupower/lib/powercap.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/power/cpupower/lib/powercap.c > > b/tools/power/cpupower/lib/powercap.c > > index 0ce29ee4c2e4..a39ee1c89679 100644 > > --- a/tools/power/cpupower/lib/powercap.c > > +++ b/tools/power/cpupower/lib/powercap.c > > @@ -51,7 +51,7 @@ static int sysfs_get_enabled(char *path, int > > *mode) > > close(fd); > > return -1; > > }> - > > + close(fd); > > The error path logic can be simplified with a goto to > to handle the error path to close the file and return. > > > if (yes_no == '1') { > > *mode = 1; > > return 0; > > This path that returns 0 can be simplified as well to do > return just once, after error path handling is done with > a goto. Ok, I will provide V2 > > Please send me v2 with the changes above with information > on how you found the problem. > I didn't find this problem by testing, just by reading the code > thanks, > -- Shuah > > thanks, > -- Shuah
diff --git a/tools/power/cpupower/lib/powercap.c b/tools/power/cpupower/lib/powercap.c index 0ce29ee4c2e4..a39ee1c89679 100644 --- a/tools/power/cpupower/lib/powercap.c +++ b/tools/power/cpupower/lib/powercap.c @@ -51,7 +51,7 @@ static int sysfs_get_enabled(char *path, int *mode) close(fd); return -1; } - + close(fd); if (yes_no == '1') { *mode = 1; return 0;
The sysfs_get_enabled() opened file processor not closed, may cause a file handle leak. Signed-off-by: Hao Zeng <zenghao@kylinos.cn> --- tools/power/cpupower/lib/powercap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)