fclose vs ferror (from libc/getcap)
As mentioned in pr bin/22965, I stumbled across a bug
in libc/gen/getcap.c when compiling it under other
platforms. The basic problem is some code which does:
(void)fclose(pfp);
if (ferror(pfp)) {
...do stuff...
}
I find it surprising that the above works under FreeBSD.
(not only that, it seems to work OK under several other
OS's too). The fclose is supposed to throw away the
stream, but the ferror (apparently) still works with
that pointer.
The PR includes a patch for getcap.c (which is just to
use the result from fclose to determine if an error
occurred), but I also wonder if we should do something
so that this combination would ALWAYS fail. It seems
to me that fclose should clear out whatever fields that
ferror is using for sanity-checking, and ferror should
always return an errno of 'bad parameter', or something
like that.
Or is there some reason that we DO want ferror to work
on streams which have already been closed?
--
---
Garance Alistair Drosehn= [EMAIL PROTECTED]
Senior Systems Programmer or [EMAIL PROTECTED]
Rensselaer Polytechnic Instituteor [EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
Garance A Drosihn wrote:
> As mentioned in pr bin/22965, I stumbled across a bug
> in libc/gen/getcap.c when compiling it under other
> platforms. The basic problem is some code which does:
>
>(void)fclose(pfp);
>if (ferror(pfp)) {
>...do stuff...
>}
>
> I find it surprising that the above works under FreeBSD.
> (not only that, it seems to work OK under several other
> OS's too). The fclose is supposed to throw away the
> stream, but the ferror (apparently) still works with
> that pointer.
>
> The PR includes a patch for getcap.c (which is just to
> use the result from fclose to determine if an error
> occurred), but I also wonder if we should do something
> so that this combination would ALWAYS fail. It seems
> to me that fclose should clear out whatever fields that
> ferror is using for sanity-checking, and ferror should
> always return an errno of 'bad parameter', or something
> like that.
>
> Or is there some reason that we DO want ferror to work
> on streams which have already been closed?
Bear in mind that ferror is one of a number of stdio functions that
are often implemented as macros. For instance: ferror and (say)
getc might look like this:
#define ferror(f) ((f)->fl & _FERR)
#define getc(f) ((f)->p < (f)->xr ? *(f)->p++ : fgetc(f))
Given the intentionally minimalist way those functions are written,
to do any consistent and obligatory sanity-checking on (FILE *) would
cause a big change in the actual code generated, and the amount of
code generated.
I think the best way to do what you want is to create a separate
debugging library.
The point about
(void)fclose(pfp);
if (ferror(pfp)) {
...do stuff...
}
is that it's a silly thing to do deliberately, but if I was porting
some hairy old C code I'd tend to expect it to work. C is not a
language in which you go out of your way to prevent people making
mistakes.
--
Robert Nordier
[EMAIL PROTECTED]
[EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
At 12:19 PM +0200 11/20/00, Robert Nordier wrote:
>Garance A Drosihn wrote:
> > [...] The basic problem is some code which does:
> >
>> (void)fclose(pfp);
>> if (ferror(pfp)) {
>> ...do stuff...
>> }
>>
> > I find it surprising that the above works under FreeBSD.
> > [...]
>
>Bear in mind that ferror is one of a number of stdio functions that
>are often implemented as macros. For instance: ferror and (say)
>getc might look like this:
>
> #define ferror(f) ((f)->fl & _FERR)
> #define getc(f) ((f)->p < (f)->xr ? *(f)->p++ : fgetc(f))
>
>Given the intentionally minimalist way those functions are written,
>to do any consistent and obligatory sanity-checking on (FILE *) would
>cause a big change in the actual code generated, and the amount of
>code generated.
Hmm. I can see that it would add some overhead. This may seem
paradoxical, but I'm not quite as concerned about getc as I am
about ferror. If you're calling getc on a closed stream, then
you're almost always going to get into some obvious trouble, and
right in that section of code. The thing with ferror is that it
will generally "work" after the fclose, although the value it
returns might not be the right (pre-close) value.
And before I go on with the rest of my response, let me note that
I realize that my own (personal) headaches with this is due to
linux's implementation of ferror. It's just that this was in code
from freebsd that I was trying to run on linux, and I was surprised
that this fclose/ferror combination was not a problem caught on
freebsd.
>I think the best way to do what you want is to create a separate
>debugging library.
This is great if you already know where the problem is. In my
case, I started out with a problem that was nowhere-near the
code which had the above bug in it. Initially the process was
dying in a call to inet_aton(). I commented that out (as a
debugging measure, since I knew what the result of that call
would be), and then the process would die in some other library
routine that I forget right now. I commented out the call to
THAT routine, and finally the process started to die on a call
to fopen. Not an fopen anywhere near the above fclose, but at
least this indicated the problem might have something to do with
io-streams. To make debugging matters worse, I am debugging
this in a daemon process, and the process would sometimes work
perfectly fine, while other times it would simply disappear.
For those and other reasons, I had spent a few hours debugging
the problem before I had any idea the problem was specific to
routines for streamed IO.
[note that the debugging library does not do any good unless one
knows to #undef those macros, so that strategy does not work
until after someone has already figured out where the problem is]
>The point about
>
>(void)fclose(pfp);
>if (ferror(pfp)) {
>...do stuff...
>}
>
>is that it's a silly thing to do deliberately, but if I was
>porting some hairy old C code I'd tend to expect it to work.
>C is not a language in which you go out of your way to prevent
>people making mistakes.
I would not expect it to work. This has nothing to do with
the C language, it has to do with fclose. Fclose gets rid
of the descriptor. In my own code, I usually follow 'fclose(fp)'
with 'fp = NULL', because that stream is GONE. I do realize that
this code does seem to work on several operating systems, but it
also causes dramatic problems with linux. Given the description
of fclose, I'd say it is the code which is wrong.
The "single Unix spec" says:
After the call to fclose(), any use of stream causes
undefined behavior.
FreeBSD's own man page for fclose says:
[fclose returns 0 or EOF]. In either case, no further
access to the stream is possible.
Neither of those indicate that anyone should "expect it to
work", no matter what language they are programming in. There
is nothing in the description for ferror which implies that it
is some magical exception to the above rules.
I might grudgingly admit that it would be a shame to
increase the overhead of macro-ized 'ferror' calls, and I'm
not sure of any good way to avoid that. But I see no reason
that this code should be "expected" to work. And if it does
not work, then I'd rather it failed right at the call to
ferror, and not some indeterminate place later on. (obviously
this is the big problem with the implementation of ferror on
linux, in that it doesn't fail or die at the ferror call, it
dies much much later due to some corrupted data structure).
Perhaps it would be helpful to add some sanity checking in the
subroutine-ized version of ferror, in libc/lib/stdio/ferror.c ?
(although I realize that wouldn't have really helped me at all)
Really I should be bugging someone in linux-land about this,
as it is their implementation which causes such painfully
obscure problems. It's just that I don't see myself as a
linux devel
Re: fclose vs ferror (from libc/getcap)
Garance A Drosihn wrote: > > The "single Unix spec" says: > After the call to fclose(), any use of stream causes > undefined behavior. > > FreeBSD's own man page for fclose says: > [fclose returns 0 or EOF]. In either case, no further > access to the stream is possible. > > Neither of those indicate that anyone should "expect it to > work", no matter what language they are programming in. There > is nothing in the description for ferror which implies that it > is some magical exception to the above rules. Undefined behavior means anything goes. On a standard, it means the behaviour is implementation-defined (which may be undefined or not). And notice that ferror() is not an access to the stream. -- Daniel C. Sobral(8-DCS) [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] "All right, Lieutenant, let's see what you do know. Whatever it is, it's not enough, but at least you haven't done anything stupid yet." "I've hardly had time, sir." "There's a naive statement." To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
At 2:11 PM +0900 11/22/00, Daniel C. Sobral wrote: >Garance A Drosihn wrote: > > > > The "single Unix spec" says: >>After the call to fclose(), any use of stream causes >>undefined behavior. >> >> FreeBSD's own man page for fclose says: >>[fclose returns 0 or EOF]. In either case, no further >>access to the stream is possible. >> >> Neither of those indicate that anyone should "expect it to >> work", no matter what language they are programming in. There >> is nothing in the description for ferror which implies that it >> is some magical exception to the above rules. > >Undefined behavior means anything goes. On a standard, it means the >behaviour is implementation-defined (which may be undefined or not). But if "anything goes", then that you can not expect it to work; certainly not when porting to other platforms. I am not saying that what freebsd does is wrong. But Robert said that "[that code is a silly thing], but if I was porting some hairy old C code I'd tend to expect it to work." It seems to me that if the behavior is explicitly undefined then you can NOT expect much of anything when porting. In any case, I did bug redhat about it (once I found the right web page), as it is the implementation of fclose in glibc which really caused all my headaches. Someone has already replied, at least to change the bug report from libc to glibc. As for freebsd, I do hope to fix the specific code in getcap.c, but I am not suggesting a change to fclose/ferror unless other developers felt this was a problem (and obviously no one seems to :-). I did want to remark on it, in case other people USE the combination of fclose followed by ferror, and then run into mysterious problems when they port their code to other platforms. I certainly did not enjoy tracking this down, and by mentioning it here then maybe I'd save someone else some headaches. > And notice that ferror() is not an access to the stream. In the section I quoted from unix spec, "stream" refers to the variable passed to fclose (though that isn't obvious, because I didn't copy the formatting). ferror certainly does access that variable. -- --- Garance Alistair Drosehn= [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Instituteor [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
Garance A Drosihn wrote: > > >Undefined behavior means anything goes. On a standard, it means the > >behaviour is implementation-defined (which may be undefined or not). > > But if "anything goes", then that you can not expect it to > work; certainly not when porting to other platforms. Sure enough. The behavior of any function on a closed FILE* is not defined by standards. > I am not saying that what freebsd does is wrong. But Robert > said that "[that code is a silly thing], but if I was porting > some hairy old C code I'd tend to expect it to work." > It seems to me that if the behavior is explicitly undefined > then you can NOT expect much of anything when porting. He said he would _tend_ to expect it to work. To me, that means the code is dubious, but is likely to work. > > And notice that ferror() is not an access to the stream. > > In the section I quoted from unix spec, "stream" refers to the > variable passed to fclose (though that isn't obvious, because I > didn't copy the formatting). ferror certainly does access that > variable. MM. That's a dubious interpretation. The variable is a handle to the stream, not the stream itself. Are you sure of the SUS wording? -- Daniel C. Sobral(8-DCS) [EMAIL PROTECTED] [EMAIL PROTECTED] [EMAIL PROTECTED] "All right, Lieutenant, let's see what you do know. Whatever it is, it's not enough, but at least you haven't done anything stupid yet." "I've hardly had time, sir." "There's a naive statement." To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
Garance A Drosihn wrote:
> >The point about
> >
> > (void)fclose(pfp);
> > if (ferror(pfp)) {
> > ...do stuff...
> > }
> >
> >is that it's a silly thing to do deliberately, but if I was
> >porting some hairy old C code I'd tend to expect it to work.
> >C is not a language in which you go out of your way to prevent
> >people making mistakes.
>
> I would not expect it to work. This has nothing to do with
> the C language, it has to do with fclose. Fclose gets rid
> of the descriptor. In my own code, I usually follow 'fclose(fp)'
> with 'fp = NULL', because that stream is GONE. I do realize that
> this code does seem to work on several operating systems, but it
> also causes dramatic problems with linux. Given the description
> of fclose, I'd say it is the code which is wrong.
This has to do with the C language at least in the sense that the
standard library is a part of C. About half the bulk of the original
ANSI/ISO C Standard is taken up with specifying the library.
I don't think I really disagree with the points you make, mostly
not quoted; but I also think the fundamental issue is that you're
not entirely in sympathy with the C way of doing things, and that
you'd prefer to be using something else. (Someone who does the
fp = NULL thing is quite likely deep-down a Modula-3 guy, or an
Oberon guy, or an Ada guy, or something. :-)
The _Rationale_, which was put out by the original ANSI committee
somewhat in defence of the C Standard itself, talks about sticking
to "the spirit of C". And the first of the tenets it mentions is
"Trust the programmer".
It's probably fair to say that the lack of sanity-checking in the
routines of the standard library is one example of that kind of
trust. Though whether it is sensible to trust the programmer not
to make silly mistakes is a different matter altogether, of course.
--
Robert Nordier
[EMAIL PROTECTED]
[EMAIL PROTECTED]
To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
Daniel C. Sobral wrote: > Garance A Drosihn wrote: > > > > >Undefined behavior means anything goes. On a standard, it means the > > >behaviour is implementation-defined (which may be undefined or not). While not disagreeing with what I think Daniel means: at least in the C Standard itself, "undefined behavior" and "implementation-defined behavior" are kept carefully separate. A quote from X3J11's Rationale document probably addresses the crux of the issue Garance raised, though: Undefined behavior gives the implementor license not to catch certain program errors > > I am not saying that what freebsd does is wrong. But Robert > > said that "[that code is a silly thing], but if I was porting > > some hairy old C code I'd tend to expect it to work." > > > It seems to me that if the behavior is explicitly undefined > > then you can NOT expect much of anything when porting. > > He said he would _tend_ to expect it to work. To me, that means the code > is dubious, but is likely to work. Garance himself writes, two messages back: The thing with ferror is that it will generally "work" after the fclose, although the value it returns might not be the right (pre-close) value. and I really meant something very similar. Though I'd probably go a bit further and say -- knowing how ferror is likely to be implemented -- the most probable result of invoking it after fclose would be a failure to detect that an I/O error had occurred. For "hairy old C code", that "works" about as much as I'd expect it to. > > > And notice that ferror() is not an access to the stream. > > > > In the section I quoted from unix spec, "stream" refers to the > > variable passed to fclose (though that isn't obvious, because I > > didn't copy the formatting). ferror certainly does access that > > variable. > > MM. That's a dubious interpretation. The variable is a handle to the > stream, not the stream itself. Are you sure of the SUS wording? The original ISO standard defines "FILE" as an object capable of recording all the information needed to control a stream [7.9.1] and elsewhere goes on to describe a stream as, for example an ordered sequence of characters [7.9.2] so I think it's fairly clear that a stream is not what (FILE *) points at. I doubt that the SUS would intentionally deviate on such a fundamental point. -- Robert Nordier [EMAIL PROTECTED] [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
When you look at the fclose()/ferror() problem you have to look at it in its historical context. Historically some versions of UNIX had very odd semantics. For example, many programmers depended on free()'d data being left intact at least until the next free(). It was even documented to have that behavior at one time (though I forget where, this was a long time ago). Similarly, close() didn't always return an integer... it used to be a void function on many systems, or return garbage (i.e. be mis-implemented) and thus undependable. And fclose() also used to be a void function on many systems or have an undependable return value. Enough confusion occured from these differences that some programmers often took liberties way back then that are obivously illegal today.. for example, calling ferror() after fclose() (because fclose() didn't used to return the final flush status), rather then using the more portable fflush/ferror/fclose combination. Another example is freopen()ing an fclose()'d file, especially in regards to stdin, stdout, and stderr. This confusion is further exasperated by shortcuts taken inside libc itself... for example, libc declares stdin, stdout and stderr as static structures and the exit code just assumes those pointers point to good memory, even if the streams have been closed. Many programmers use library code as a basis for learning the APIs and mistakenly come to believe that they can make similar assumptions externally that the library makes internally. In today's world the standard is: When you free() something, that's it.. it's gone. When you fclose() something or otherwise terminate a structure, it's gone. Anything else is illegal. *internally* our libc assumes that ferror() is legal after an fclose() because, well, it's true... but only for internal library functions. Nobody outside the library can legally make that assumption and it could also be argued that even within the library those types of assumptions should not be made unless absolutely necessary. There isn't much we can do about the issue except fix the instances of mis-programming as they show up. -Matt To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
At 2:25 PM +0200 11/22/00, Robert Nordier wrote: >Daniel C. Sobral wrote: > > > Garance A Drosihn wrote: > > > In the section I quoted from unix spec, "stream" refers to the >> > variable passed to fclose (though that isn't obvious, because I >> > didn't copy the formatting). ferror certainly does access that >> > variable. >> > > MM. That's a dubious interpretation. The variable is a handle > > to the stream, not the stream itself. Are you sure of the SUS > > wording? > >The original ISO standard defines "FILE" as > > an object capable of recording all the information needed to > control a stream [7.9.1] > >[...skipping...] > >I doubt that the SUS would intentionally deviate on such a >fundamental point. I've replied to both Robert and Daniel with (I think) the exact section of SUS that I was quoting. I didn't want to send a message with all those formatting characters to the mailing list, because I really don't know how well that would work out... For others who might be curious, I was not saying that "the concept of a stream" refers to the variable sent to fclose. I meant that in the exact section I was quoting, that word 'stream' is in italics, and thus it refers to the variable 'stream' which SUS used in the synopsis of fclose. Eg: #include int fclose(FILE *stream); -- (the underlined 'stream' was written in italics), and: After the call to fclose(), any use of stream -- -- causes undefined behaviour. (where both fclose and stream are in italics in the section I was quoting from). Their description for fclose does also use the phrase 'a stream' (with no italics) in the same way everyone else has been using it. - - - - - - Also for the curious, here is the "resolution" of my bug report as it was sent to redhat: + libc is allowed to start nethack, format your disks, + whatever it wants in this case. + + The fact that it magically works on some other system + means nothing, the results of such operation are undefined. + glibc in ferror has to acquire lock of the stream in + question first (thus writes into memory). Perhaps other + systems either don't care about multiple threads (and do + no locking) or slow each operation down (by checking + if the file descriptor is valid at the start of every + single routine). + + You can turn some of such checks by recompiling glibc + with IO_DEBUG, but as such checks just catch some cases + and can pass even on invalid FILE descriptors and also + slow things down, they are not enabled by default. + + So think about ferror on fclosed FILE as if you put + random garbage into that memory area yourself. It is interesting that they talk about acquiring a lock for the stream in question first. As freebsd does more with SMP and threads, will freebsd need to do similar locking? They are also trumpeting the fact that the behavior in this case is "undefined", and thus they feel there is no problem with the fact that the call to ferror trashes random locations in memory and will trigger very obscure bugs which are no where near the "real bug" (ie, the ferror). All very well. Everyone gets to use the term "undefined behavior" to justify that no changes should be made to there version fclose or ferror. All I'm saying is that this leaves the person porting "hairy old C code" in a mighty unpleasant position. This fclose/ferror combination is a fairly easy mistake to miss (particularly since it does work on freebsd), but it might be lethal on other platforms. And if it is lethal, you will get no sympathy once you do track it down. Here I lost 30 hours tracking down a bug in code I did not write in the first place, and everyone's attitude seems to be that it's my fault for wanting to port some C code to multiple platforms. So, for those hackers who do port C code, put this combination on the list of things you should lookout for. -- --- Garance Alistair Drosehn= [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Instituteor [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
At 9:36 AM -0800 11/22/00, Matt Dillon wrote: > [...] When you fclose() something or otherwise terminate a > structure, it's gone. Anything else is illegal. *internally* > our libc assumes that ferror() is legal after an fclose() > because, well, it's true... but only for internal library > functions. Nobody outside the library can legally make that > assumption and it could also be argued that even within the > library those types of assumptions should not be made unless > absolutely necessary. Hmm. That does bring up an important point. The code with the fclose/ferror combination *is* something I was taking directly out of libc. So, it would have more right than most code to make explicit assumptions about the implementation of other libc routines. I had not thought of it in that way, mainly because I pulled it out of libc at least five years ago, and it didn't cause me any trouble until this month. > There isn't much we can do about the issue except fix the > instances of mis-programming as they show up. Yep. Oh well. On to the next tempest, please pass the tea. -- --- Garance Alistair Drosehn= [EMAIL PROTECTED] Senior Systems Programmer or [EMAIL PROTECTED] Rensselaer Polytechnic Instituteor [EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
Re: fclose vs ferror (from libc/getcap)
Matt Dillon wrote: > > [...] > > In today's world the standard is: When you free() something, that's it.. > it's gone. When you fclose() something or otherwise terminate a > structure, it's gone. Anything else is illegal. *internally* our libc > assumes that ferror() is legal after an fclose() because, well, it's > true... but only for internal library functions. Nobody outside the > library can legally make that assumption and it could also be argued > that even within the library those types of assumptions should not be > made unless absolutely necessary. > > There isn't much we can do about the issue except fix the instances > of mis-programming as they show up. An excellent summary of what is and what should be. If we wanted to be anally retentive, we could bzero the FILE after completing the close() operation, just to "help" poorly written programs core... -- "Where am I, and what am I doing in this handbasket?" Wes Peters Softweyr LLC [EMAIL PROTECTED] http://softweyr.com/ To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-hackers" in the body of the message
