* [Bug libstdc++/17215] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
@ 2004-08-27 18:07 ` ken at xorian dot net
2004-08-27 19:25 ` pcarlini at suse dot de
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: ken at xorian dot net @ 2004-08-27 18:07 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From ken at xorian dot net 2004-08-27 18:07 -------
Created an attachment (id=6996)
--> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=6996&action=view)
Propsed patch
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
2004-08-27 18:07 ` [Bug libstdc++/17215] " ken at xorian dot net
@ 2004-08-27 19:25 ` pcarlini at suse dot de
2004-08-27 22:49 ` ken at xorian dot net
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pcarlini at suse dot de @ 2004-08-27 19:25 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From pcarlini at suse dot de 2004-08-27 19:25 -------
Hi,
I think that basically both the problem is real and your solution correct,
thanks. However, a few details:
> __basic_file<char>::close ignores the return value of fclose(3). This
> call can fail and set errno to a variety of values (ENOSPC, EINTR,
> etc.).
Notice that the C standard, the reference for the samantics of fclose in C++,
tells nothing about errno, see C99, 7.19.5.1.
> Aside from not notifying the client code (e.g. by setting
> failbit in an enclosing fstream),
Agreed.
> this can also cause a memory leak as
> the FILE data structure won't be de-allocated by a failed fclose and
> __basic_file<char>::close discards it regardless.
I doubt it. Again, see 7.19.5.1: the semantics of fclose in the C standard
is not the same of close (in Posix, or what else): 7.19.5.1, p2, clearly
points out that even when fclose fails there are no risks of leaks, since
the buffers are deallocated anyway. Also, the stream is disassociated from
the file and you cannot perform additional operations on it. All of this
implies, in my opinion, that your fix should be tweaked to do _M_cfile = 0
unconditionally.
By the way, is your patch already regression tested? Can you think of any
testcases?
Thanks,
Paolo.
--
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |WAITING
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
2004-08-27 18:07 ` [Bug libstdc++/17215] " ken at xorian dot net
2004-08-27 19:25 ` pcarlini at suse dot de
@ 2004-08-27 22:49 ` ken at xorian dot net
2004-08-28 18:32 ` pcarlini at suse dot de
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: ken at xorian dot net @ 2004-08-27 22:49 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From ken at xorian dot net 2004-08-27 22:49 -------
> I doubt it. Again, see 7.19.5.1: the semantics of fclose in the C
> standard is not the same of close (in Posix, or what else):
> 7.19.5.1, p2, clearly points out that even when fclose fails there
> are no risks of leaks, since the buffers are deallocated
> anyway. Also, the stream is disassociated from the file and you
> cannot perform additional operations on it. All of this implies, in
> my opinion, that your fix should be tweaked to do _M_cfile = 0
> unconditionally.
I admit I had not checked C99 on this, and I find what you point out
to be rather distressing actually. Several other references (the
Single UNIX Specification, man pages on Linux and Tru64) say that
fclose can fail with errno=EINTR. I had assumed that in the event of
that kind of temporary failure (where the normal thing to do is to
repeat the operation) that one would be able to call fclose on the
same stream to complete the write(2) or close(2) that had been
interrupted.
If a C run-time library were to implement fclose as C99 seems to
describe, then such a transient error would be totally unrecoverable.
That would make stdio and by extension std::fstream unusable for any
application that required data integrity.
It may be worth mentioning that I noticed the problem this bug report
is about because I had fstream::open fail with errno=EINTR. I've been
trying to identify all the changes I need to make to at the
application level to properly protect against this causing spurious
failures. Since fstream::open can fail in this way, it seemed
sensible to add the same protection and repeating of the operation on
EINTR around fstream::close. (I did check the C++ spec, which states
that basic_filebuf::close will fail is fclose fails.)
> By the way, is your patch already regression tested? Can you think
> of any testcases?
I did not run any of the regression tests included with gcc on it,
though I've done some very minimal testing using it with some code I'm
working with.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (2 preceding siblings ...)
2004-08-27 22:49 ` ken at xorian dot net
@ 2004-08-28 18:32 ` pcarlini at suse dot de
2004-08-30 11:34 ` cvs-commit at gcc dot gnu dot org
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pcarlini at suse dot de @ 2004-08-28 18:32 UTC (permalink / raw)
To: gcc-bugs
--
What |Removed |Added
----------------------------------------------------------------------------
AssignedTo|unassigned at gcc dot gnu |pcarlini at suse dot de
|dot org |
Status|WAITING |ASSIGNED
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (3 preceding siblings ...)
2004-08-28 18:32 ` pcarlini at suse dot de
@ 2004-08-30 11:34 ` cvs-commit at gcc dot gnu dot org
2004-08-30 11:35 ` pcarlini at suse dot de
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: cvs-commit at gcc dot gnu dot org @ 2004-08-30 11:34 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From cvs-commit at gcc dot gnu dot org 2004-08-30 11:34 -------
Subject: Bug 17215
CVSROOT: /cvs/gcc
Module name: gcc
Changes by: paolo@gcc.gnu.org 2004-08-30 11:33:54
Modified files:
libstdc++-v3 : ChangeLog
libstdc++-v3/config/io: basic_file_stdio.cc
Log message:
2004-08-30 Paolo Carlini <pcarlini@suse.de>
Kenneth C. Schalk <ken@xorian.net>
PR libstdc++/17215
* config/io/basic_file_stdio.cc (__basic_file<char>::close()):
Check the return value of fclose/sync, loop on EINTR.
(__basic_file<char>::sys_open): Likewise, for sync.
Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2646&r2=1.2647
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/io/basic_file_stdio.cc.diff?cvsroot=gcc&r1=1.32&r2=1.33
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (4 preceding siblings ...)
2004-08-30 11:34 ` cvs-commit at gcc dot gnu dot org
@ 2004-08-30 11:35 ` pcarlini at suse dot de
2004-09-02 17:41 ` [Bug libstdc++/17215] [3.4 only] " bkoz at gcc dot gnu dot org
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pcarlini at suse dot de @ 2004-08-30 11:35 UTC (permalink / raw)
To: gcc-bugs
--
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |3.4.3
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] [3.4 only] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (5 preceding siblings ...)
2004-08-30 11:35 ` pcarlini at suse dot de
@ 2004-09-02 17:41 ` bkoz at gcc dot gnu dot org
2004-09-02 18:09 ` ken at xorian dot net
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: bkoz at gcc dot gnu dot org @ 2004-09-02 17:41 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From bkoz at gcc dot gnu dot org 2004-09-02 17:41 -------
There really should be a test case for this in the testsuite. Kenneth, can you
post the code you used to notice this?
thanks,
benjamin
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] [3.4 only] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (6 preceding siblings ...)
2004-09-02 17:41 ` [Bug libstdc++/17215] [3.4 only] " bkoz at gcc dot gnu dot org
@ 2004-09-02 18:09 ` ken at xorian dot net
2004-11-01 0:45 ` mmitchel at gcc dot gnu dot org
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: ken at xorian dot net @ 2004-09-02 18:09 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From ken at xorian dot net 2004-09-02 18:09 -------
Unfortunately, I haven't had much luck with generating a "real" EINTR.
I've tried various methods (such as forking a separate process that
sits in a loop killing the main process with a signal whose handler
does nothing), but I haven't come up with a method that works.
The way I noticed it originally was with a method for "faking" EINTR.
I'll attach the single C file (fake_eintr.c) which can be compiled
into a shared library like so:
gcc -shared -o fake_eintr.so fake_eintr.c -ldl -lc
And used as an LD_PRELOAD (on Linux at least). This is, however,
unsatisfying as a method of testing this as it doesn't reflect state
changes that would happen inside the C run-time library when a real
EINTR occurs. (For example, ferror(3) would return false after a
faked EINTR from my flose(3) wrapper.)
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] [3.4 only] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (7 preceding siblings ...)
2004-09-02 18:09 ` ken at xorian dot net
@ 2004-11-01 0:45 ` mmitchel at gcc dot gnu dot org
2004-11-08 0:41 ` cvs-commit at gcc dot gnu dot org
2004-11-08 0:42 ` pcarlini at suse dot de
10 siblings, 0 replies; 12+ messages in thread
From: mmitchel at gcc dot gnu dot org @ 2004-11-01 0:45 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From mmitchel at gcc dot gnu dot org 2004-11-01 00:45 -------
Postponed until GCC 3.4.4.
--
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|3.4.3 |3.4.4
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] [3.4 only] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (8 preceding siblings ...)
2004-11-01 0:45 ` mmitchel at gcc dot gnu dot org
@ 2004-11-08 0:41 ` cvs-commit at gcc dot gnu dot org
2004-11-08 0:42 ` pcarlini at suse dot de
10 siblings, 0 replies; 12+ messages in thread
From: cvs-commit at gcc dot gnu dot org @ 2004-11-08 0:41 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From cvs-commit at gcc dot gnu dot org 2004-11-08 00:41 -------
Subject: Bug 17215
CVSROOT: /cvs/gcc
Module name: gcc
Branch: gcc-3_4-branch
Changes by: paolo@gcc.gnu.org 2004-11-08 00:41:15
Modified files:
libstdc++-v3 : ChangeLog
libstdc++-v3/config/io: basic_file_stdio.cc
libstdc++-v3/include/bits: fstream.tcc
Log message:
2004-11-07 Paolo Carlini <pcarlini@suse.de>
Andrea Arcangeli <andrea@suse.de>
* config/io/basic_file_stdio.cc (__basic_file<>::close)): Don't
call unnecessarily sync, that is fflush: the library, since 3.4.0
does not use buffered fread/fwrite.
* include/bits/fstream.tcc (basic_filebuf<>::overflow): Likewise.
2004-11-07 Paolo Carlini <pcarlini@suse.de>
Kenneth C. Schalk <ken@xorian.net>
PR libstdc++/17215
* config/io/basic_file_stdio.cc (__basic_file<char>::close()):
Check the return value of fclose/sync, loop on EINTR.
(__basic_file<char>::sys_open): Likewise, for sync.
Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2224.2.194&r2=1.2224.2.195
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/config/io/basic_file_stdio.cc.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.25.4.6&r2=1.25.4.7
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/fstream.tcc.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.116.4.7&r2=1.116.4.8
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug libstdc++/17215] [3.4 only] __basic_file<char>::close ignores errors
2004-08-27 18:04 [Bug libstdc++/17215] New: __basic_file<char>::close ignores errors ken at xorian dot net
` (9 preceding siblings ...)
2004-11-08 0:41 ` cvs-commit at gcc dot gnu dot org
@ 2004-11-08 0:42 ` pcarlini at suse dot de
10 siblings, 0 replies; 12+ messages in thread
From: pcarlini at suse dot de @ 2004-11-08 0:42 UTC (permalink / raw)
To: gcc-bugs
------- Additional Comments From pcarlini at suse dot de 2004-11-08 00:42 -------
Fixed for 3.4.4.
--
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17215
^ permalink raw reply [flat|nested] 12+ messages in thread