[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Feiyu Zhu]

ltp-pan will leak file descriptors of fopen() into the child process
of the test case, fix this problem by using mode "e" for fopen().

Signed-off-by: Feiyu Zhu <zhufy.jy@cn.fujitsu.com>
---
 pan/ltp-pan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pan/ltp-pan.c b/pan/ltp-pan.c
index 8b9fbe5..25e554f 100644
--- a/pan/ltp-pan.c
+++ b/pan/ltp-pan.c
@@ -336,7 +336,7 @@ int main(int argc, char **argv)
 		if (!strcmp(logfilename, "-")) {
 			logfile = stdout;
 		} else {
-			if ((logfile = fopen(logfilename, "a+")) == NULL) {
+			if ((logfile = fopen(logfilename, "a+e")) == NULL) {
 				fprintf(stderr,
 					"pan(%s): Error %s (%d) opening log file '%s'\n",
 					panname, strerror(errno), errno,
@@ -453,7 +453,7 @@ int main(int argc, char **argv)
 	}
 
 	if (failcmdfilename) {
-		if (!(failcmdfile = fopen(failcmdfilename, "a+"))) {
+		if (!(failcmdfile = fopen(failcmdfilename, "a+e"))) {
 			fprintf(stderr,
 				"pan(%s): Error %s (%d) opening fail cmd file '%s'\n",
 				panname, strerror(errno), errno,
@@ -463,7 +463,7 @@ int main(int argc, char **argv)
 	}
 
 	if (tconfcmdfilename) {
-		tconfcmdfile = fopen(tconfcmdfilename, "a+");
+		tconfcmdfile = fopen(tconfcmdfilename, "a+e");
 		if (!tconfcmdfile) {
 			fprintf(stderr, "pan(%s): Error %s (%d) opening "
 				"tconf cmd file '%s'\n", panname,
-- 
1.8.3.1

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Cyril Hrubis]

Hi!
> ltp-pan will leak file descriptors of fopen() into the child process
> of the test case, fix this problem by using mode "e" for fopen().

Looks good, however this is supported since glibc 2.7 and it does not
seem to be supported on musl libc either.

I guess that it would be better just to close these files after a fork
in the runchild() function, but that would mean that we would have to
pass all these files as paramters to the function as well.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Yang Xu]

Hi Cyril, Feiyu
> Hi!
>> ltp-pan will leak file descriptors of fopen() into the child process
>> of the test case, fix this problem by using mode "e" for fopen().
>
> Looks good, however this is supported since glibc 2.7 and it does not
> seem to be supported on musl libc either.
>
Yes, musl-libc doesn't support "e" mode for fopen[1].

[1]https://git.musl-libc.org/cgit/musl/tree/src/stdio/fopen.c
> I guess that it would be better just to close these files after a fork
> in the runchild() function, but that would mean that we would have to
> pass all these files as paramters to the function as well.
+1
>

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Yang Xu]

Hi Cyril
> Hi Cyril, Feiyu
>> Hi!
>>> ltp-pan will leak file descriptors of fopen() into the child process
>>> of the test case, fix this problem by using mode "e" for fopen().
>>
>> Looks good, however this is supported since glibc 2.7 and it does not
>> seem to be supported on musl libc either.
>>
> Yes, musl-libc doesn't support "e" mode for fopen[1].
Sorry, I ignore __fmodeflags function, musl libc supports "e" mode since 
0.9.7 after this commit 8582a6e9f ("add 'e' modifier (close-on-exec) to 
fopen and fdopen").

https://git.musl-libc.org/cgit/musl/commit/src?id=8582a6e9f25dd7b87d72961f58008052a4cac473

It is about 8 years since musl libc fopen() supports "e". glibc2.7 
fopen() supports "e" is about 13 years.  Maybe we can use "e" mode now?
>
> [1]https://git.musl-libc.org/cgit/musl/tree/src/stdio/fopen.c
>> I guess that it would be better just to close these files after a fork
>> in the runchild() function, but that would mean that we would have to
>> pass all these files as paramters to the function as well.
> +1
>>
>
>
>
>

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Cyril Hrubis]

Hi!
> > Yes, musl-libc doesn't support "e" mode for fopen[1].
> Sorry, I ignore __fmodeflags function, musl libc supports "e" mode since 
> 0.9.7 after this commit 8582a6e9f ("add 'e' modifier (close-on-exec) to 
> fopen and fdopen").
> 
> https://git.musl-libc.org/cgit/musl/commit/src?id=8582a6e9f25dd7b87d72961f58008052a4cac473
> 
> It is about 8 years since musl libc fopen() supports "e". glibc2.7 
> fopen() supports "e" is about 13 years.  Maybe we can use "e" mode now?

To be honest I haven't had used ltp-pan for last two years, so if that
change works for everyone still using it, then we can go ahead with it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Yang Xu]

Hi Cyril
> Hi!
>>> Yes, musl-libc doesn't support "e" mode for fopen[1].
>> Sorry, I ignore __fmodeflags function, musl libc supports "e" mode since
>> 0.9.7 after this commit 8582a6e9f ("add 'e' modifier (close-on-exec) to
>> fopen and fdopen").
>>
>> https://git.musl-libc.org/cgit/musl/commit/src?id=8582a6e9f25dd7b87d72961f58008052a4cac473
>>
>> It is about 8 years since musl libc fopen() supports "e". glibc2.7
>> fopen() supports "e" is about 13 years.  Maybe we can use "e" mode now?
>
> To be honest I haven't had used ltp-pan for last two years, so if that
> change works for everyone still using it, then we can go ahead with it.
OK. I will wait a week. If nobody has objection, I will merge it.

Best Regards
Yang Xu
>

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Yang Xu]

Hi Petr
> Hi Cyril
>> Hi!
>>>> Yes, musl-libc doesn't support "e" mode for fopen[1].
>>> Sorry, I ignore __fmodeflags function, musl libc supports "e" mode since
>>> 0.9.7 after this commit 8582a6e9f ("add 'e' modifier (close-on-exec) to
>>> fopen and fdopen").
>>>
>>> https://git.musl-libc.org/cgit/musl/commit/src?id=8582a6e9f25dd7b87d72961f58008052a4cac473
>>>
>>>
>>> It is about 8 years since musl libc fopen() supports "e". glibc2.7
>>> fopen() supports "e" is about 13 years. Maybe we can use "e" mode now?
>>
>> To be honest I haven't had used ltp-pan for last two years, so if that
>> change works for everyone still using it, then we can go ahead with it.
> OK. I will wait a week. If nobody has objection, I will merge it.
I plan to merge this patch today. Before it, I want to listen some 
advise from you( IMO, you know musl-libc a lot and other libc on 
embedded system).
>
> Best Regards
> Yang Xu
>>
>

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Petr Vorel]

Hi Xu,

> Hi Petr
> > Hi Cyril
> > > Hi!
> > > > > Yes, musl-libc doesn't support "e" mode for fopen[1].
> > > > Sorry, I ignore __fmodeflags function, musl libc supports "e" mode since
> > > > 0.9.7 after this commit 8582a6e9f ("add 'e' modifier (close-on-exec) to
> > > > fopen and fdopen").

> > > > https://git.musl-libc.org/cgit/musl/commit/src?id=8582a6e9f25dd7b87d72961f58008052a4cac473


> > > > It is about 8 years since musl libc fopen() supports "e". glibc2.7
> > > > fopen() supports "e" is about 13 years. Maybe we can use "e" mode now?

> > > To be honest I haven't had used ltp-pan for last two years, so if that
> > > change works for everyone still using it, then we can go ahead with it.
> > OK. I will wait a week. If nobody has objection, I will merge it.
> I plan to merge this patch today. Before it, I want to listen some advise
> from you( IMO, you know musl-libc a lot and other libc on embedded system).

Acked-by: Petr Vorel <pvorel@suse.cz>

Should be safe.

Kind regards,
Petr

> > Best Regards
> > Yang Xu

[LTP] [PATCH] pan/ltp-pan.c: fix file descriptors leaks

[Yang Xu]

Hi Feiyu

Pushed, thanks!

Best Regards
Yang Xu
> Hi Xu,
>
>> Hi Petr
>>> Hi Cyril
>>>> Hi!
>>>>>> Yes, musl-libc doesn't support "e" mode for fopen[1].
>>>>> Sorry, I ignore __fmodeflags function, musl libc supports "e" mode since
>>>>> 0.9.7 after this commit 8582a6e9f ("add 'e' modifier (close-on-exec) to
>>>>> fopen and fdopen").
>
>>>>> https://git.musl-libc.org/cgit/musl/commit/src?id=8582a6e9f25dd7b87d72961f58008052a4cac473
>
>
>>>>> It is about 8 years since musl libc fopen() supports "e". glibc2.7
>>>>> fopen() supports "e" is about 13 years. Maybe we can use "e" mode now?
>
>>>> To be honest I haven't had used ltp-pan for last two years, so if that
>>>> change works for everyone still using it, then we can go ahead with it.
>>> OK. I will wait a week. If nobody has objection, I will merge it.
>> I plan to merge this patch today. Before it, I want to listen some advise
>> from you( IMO, you know musl-libc a lot and other libc on embedded system).
>
> Acked-by: Petr Vorel<pvorel@suse.cz>
>
> Should be safe.
>
> Kind regards,
> Petr
>
>>> Best Regards
>>> Yang Xu
>
>
> .
>