«

Richard Harter a écrit :
> Is there any way to reliably check the legitimacy of something
> that purports to be a file pointer?

No.

This is equivalent to checking a pointer. Under windows is an API that
does that, maybe in an other systems there is something similar,
but there isn't anything 100% reliable.

Solution:

When you start the program open the log file and ensure that
it is OK to write to it. Then store it in some place.
Eliminate the FILE * argument from the log function specs.

Why should be there anyway? Do you have several log files open
at the same time?

Simplify!

> The best answer would be a
> technique that is guaranteed to work. If there be not such, a
> technique that would almost certainly work in practice.
>

IsVAlidReadPointer/IsValidWritePointer under windows...

> In addition, is there a "sanitary" version of the printf
> formatting that is publicly available. The issue here is that
> printf and friends can do all sorts of ugly things.
>

True. Do not use %s

> The context is an error exit routine. The prototype looks
> something like this:
>
> void errexit(FILE * fptr,char * fmt,...);
>
> The idea is that errexit will write an error message to the
> specified file and then call exit. It would be couth if the
> errexit routine would not crash the program. Some potential
> failure points:
>

Much better would be:

void errexit(char *function_name, char *message, int flags);

Error messages do not need to be SO nicely formatted. Just print the
location of the code, the errormessage. Flags could be something that
tells you if you should exit or not (warning/error) but it can be
eliminated.

At this point, the software is going to stop running. Do NOT
complicate things, do NOT try to have fancy formatting. Just
print the message and exit.

REMEMBER: Only software people afre going to read this. They are not end
users. And if the log file is hard to read they can write a piece of
software for better formatting of the log file

jacob

On 2010-01-13, jacob navia <> wrote:
> When you start the program open the log file and ensure that
> it is OK to write to it. Then store it in some place.
> Eliminate the FILE * argument from the log function specs.
>
> Why should be there anyway? Do you have several log files open
> at the same time?
>
> Simplify!

I agree with this part! It's a good insight.

> Much better would be:
>
> void errexit(char *function_name, char *message, int flags);
>
> Error messages do not need to be SO nicely formatted. Just print the
> location of the code, the errormessage. Flags could be something that
> tells you if you should exit or not (warning/error) but it can be
> eliminated.

I disagree with this part, though. There is a ton of stuff that is likely
to happen in log messages, all over the place. We have three options:

1. Allow format strings.
2. Make every caller do their own formatting, using snprintf and a buffer,
before handing us the message. This is error-prone at best, and will involve
MANY points at which the message could be screwed up.
3. Discourage users from including relevant data along with the error
message.

Consider the difference between:
Error: Value out of range
and
Error: Value (23) out of range (expecting 1-20).

-s
--
Copyright 2010, all wrongs reversed. Peter Seebach / usenet-
http://www.seebs.net/log/ <-- lawsuits, religion, and funny pictures
http://en.wikipedia.org/wiki/Fair_Game_(Scientology) <-- get educated!

jacob navia <> writes:

> Richard Harter a écrit :
>> Is there any way to reliably check the legitimacy of something
>> that purports to be a file pointer?
>
> No.
>
> This is equivalent to checking a pointer. Under windows is an API that
> does that, maybe in an other systems there is something similar,
> but there isn't anything 100% reliable.

I'm intrigued. What does it check?

Suppose you've opened a file with fp as the pointer and then closed it.
What does it check about fp?

If there was a safe and easy way to do this, fclose could do it to check
you don't close the file twice (source of probably my all-time hardest
to spot bug: if you took a particular (and rare) path through the
program a file was closed twice. A year or more later I added code that
caused, in even rarer circumstances, another file to be opened further
through the flow. /That/ fopen was where it crashed).
--
Online waterways route planner | http://canalplan.eu
Plan trips, see photos, check facilities | http://canalplan.org.uk

On Jan 13, 12:06*pm, jacob navia <ja...@spamsink.net> wrote:
> Richard Harter a crit :
>
> > Is there any way to reliably check the legitimacy of something
> > that purports to be a file pointer?
>
> No.
>
> This is equivalent to checking a pointer. Under windows is an API that
> does that, maybe in an other systems there is something similar,
> but there isn't anything 100% reliable.
>
>(...)
>
> IsVAlidReadPointer/IsValidWritePointer under windows...


The IsBad*Pointer() functions under Windows do not do what the OP, or
almost anyone else, really want. Worse, just using them can cause
some rather bad side effects that can lead to your program crashing.

First, they don't do what you want. They will check if the address
being pointed to is mapped into your address space. This has, a best,
a marginal relationship with storage that's been malloc'd, since the
allocator commonly requests large chunks from the OS (thus having big
unallocated parts that are mapped in virtual memory but are not valid
areas for C pointers to point to), and freed areas are often not
returned to the OS either, leaving all of those to pass
IsBad*Pointer. So those functions simply don't accomplish what you
most people hope they do.

And even if you validated a pointer to ensure it was actually pointing
to a validly allocated area of the heap (which you could do, slowly,
with the _heapwalk() function in Windows), that doesn't help you to
identify storage not allocated on the normal heap (and while whatever
a file pointer is pointing at is likely allocated from there, it's
certainly not guaranteed), and it doesn't help you verify that you're
actually pointing at an object of the correct type.

Second, if you happen to accidentally IsBad*Pointer() a stack guard
page (Windows, as most OSs, don't actually allocate real memory pages
to the whole stack until the program grows into those areas), you'll
end up eating the CPU exception that the OS uses to add a real page to
the stack - and then when you *do* grow the stack into that area,
you'll get a crash, since there will not be any memory allocated
there.

In short, unless you really know what you're doing, the IsBad*Pointer
() functions are to be avoided at all costs.

Nick a écrit :
> jacob navia <> writes:
>
>> Richard Harter a écrit :
>>> Is there any way to reliably check the legitimacy of something
>>> that purports to be a file pointer?
>> No.
>>
>> This is equivalent to checking a pointer. Under windows is an API that
>> does that, maybe in an other systems there is something similar,
>> but there isn't anything 100% reliable.
>
> I'm intrigued. What does it check?
>

It checks that the RAM starting at the given address and with the
given length is readable/writable.

> Suppose you've opened a file with fp as the pointer and then closed it.
> What does it check about fp?
>
> If there was a safe and easy way to do this, fclose could do it to check
> you don't close the file twice (source of probably my all-time hardest
> to spot bug: if you took a particular (and rare) path through the
> program a file was closed twice. A year or more later I added code that
> caused, in even rarer circumstances, another file to be opened further
> through the flow. /That/ fopen was where it crashed).


Fine, but an fclose that doesn't check its parameters is really a pile of ****...
It is not that difficult to maintain a list of open files and verify that
the file is really open before closing it.

jacob navia <> writes:
> Nick a écrit :
[...]
>> Suppose you've opened a file with fp as the pointer and then closed it.
>> What does it check about fp?
>>
>> If there was a safe and easy way to do this, fclose could do it to check
>> you don't close the file twice (source of probably my all-time hardest
>> to spot bug: if you took a particular (and rare) path through the
>> program a file was closed twice. A year or more later I added code that
>> caused, in even rarer circumstances, another file to be opened further
>> through the flow. /That/ fopen was where it crashed).
>
>
> Fine, but an fclose that doesn't check its parameters is really a
> pile of ****... It is not that difficult to maintain a list of open
> files and verify that the file is really open before closing it.

I agree that fclose() can do some checking, and that it shouldn't be
too difficult in most implementations. (I won't comment on your
characterization of an implementation that doesn't do this.)

On the other hand, it's not hard to imagine an implementation that's
built on top of some lower level, where that lower level doesn't
provide this kind of checking.

Also (somebody else mentioned this scenario), consider:

fp1 = fopen(...);
fclose(fp1);
...
fp2 = fopen(...);
/* Suppose fp2 happens to point to the same FILE object that
fp1 had pointed to.
*/
...
fclose(fp1);
/* Oops, we closed fp1 twice, but the error isn't detected
because the pointer looks valid.
*/
...
fclose(fp2);
/* This should have beeen ok, but the previous fclose clobbered
the FILE object; if fclose() performs checking, it will
report the error here rather than where it really occurred
*/

That's not to say that performing checking in fclose() isn't a good
idea (and closing a file isn't likely to be performance-critical), but
it can't catch everything.

--
Keith Thompson (The_Other_Keith) kst- <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

On 14 Jan, 08:16, c...@tiac.net (Richard Harter) wrote:
> On Wed, 13 Jan 2010 19:06:42 +0100, jacob navia
>
> <ja...@spamsink.net> wrote:
> >Richard Harter a écrit :

<snip>

> >> The context is an error exit routine. *The prototype looks
> >> something like this:
>
> >> * * void errexit(FILE * fptr,char * fmt,...);
>
> >> The idea is that errexit will write an error message to the
> >> specified file and then call exit. *It would be couth if the
> >> errexit routine would not crash the program. *Some potential
> >> failure points:
>
> >Much better would be:
>
> > * * *void errexit(char *function_name, char *message, int flags);
>
> >Error messages do not need to be SO nicely formatted. Just print the
> >location of the code, the errormessage. Flags could be something that
> >tells you if you should exit or not (warning/error) but it can be
> >eliminated.

no. I hate error messages like this with a passion. I'd like to know
what went wrong!


> >At this point, the software is going to stop running. Do NOT
> >complicate things, do NOT try to have fancy formatting. Just
> >print the message and exit.
>
> >REMEMBER: Only software people afre going to read this. They are not end
> >users. And if the log file is hard to read they can write a piece of
> >software for better formatting of the log file

but the magic formatter can't add information that isn't there. We log
messages received and sent in hex. The logfile post processor turns
these into human readable form. But you need the raw data in the first
place!

> I disagree with your "much better". *I've used it and I don't
> like it. *I want to be able to include data in the message.

yes. very very yes.


--
"Half-assed programming was a time-filler that, like knitting,
must date to the beginning of human experience."
"A Fire Upon The Deep" by Verne Vinge

Richard Harter wrote:
> On Wed, 13 Jan 2010 19:06:42 +0100, jacob navia
> <> wrote:
>
>> Richard Harter a écrit :
>>> Is there any way to reliably check the legitimacy of something
>>> that purports to be a file pointer?
>> No.

<snip>

>> Solution:
>>
>> When you start the program open the log file and ensure that
>> it is OK to write to it. Then store it in some place.
>> Eliminate the FILE * argument from the log function specs.
>
> Minor quibble: The context is an error exit function, not a log
> function.

I've got one of those as well...

> Actually, the code I use does that. The issue is that there is
> no guarantee that the file pointer is still valid when the error
> exit function is called. Any number of bad things may have
> happened in the interim; indeed something bad must have happened
> or the code wouldn't be calling the error exit function.

Mine has lots of things that should be open and should be closed, but
they might be figments of the corruption causing the crash...

> One can check by trying to put a character and then checking for
> a write error. This may crash the program which is undesirable.
>
> Is there anything better?

If it is Linux/Unix the most likely crash probably generates a SIGSEGV,
and on Windows and other OSs there might be an equivalent signal which
you could trap. So what you can do is something like...

static failstep = 0;

failstep++;

switch {
case 1: log failure
case 2: tidy up
...
}

add cases etc as required in an appropriate order, and you may need a
bit more stuff in the signal handler to re-install itself for the next
SIGSEGV.

It is far from perfect, but it does allow you to first try and abort
nicely with a log message, and gradually do less and less tidying up as
you find out what you can't do.

I would also recommend NOT freeing memory in the abort code, since if
you are crashing because the structures malloc/free use doing another
free only compounds the problem.

<snip>

>> Much better would be:
>>
>> void errexit(char *function_name, char *message, int flags);
>>
>> Error messages do not need to be SO nicely formatted. Just print the
>> location of the code, the errormessage. Flags could be something that
>> tells you if you should exit or not (warning/error) but it can be
>> eliminated.
>>
>> At this point, the software is going to stop running. Do NOT
>> complicate things, do NOT try to have fancy formatting. Just
>> print the message and exit.
>>
>> REMEMBER: Only software people afre going to read this. They are not end
>> users. And if the log file is hard to read they can write a piece of
>> software for better formatting of the log file
>
> I disagree with your "much better". I've used it and I don't
> like it. I want to be able to include data in the message.

I agree, even the programmer needs more information than can be
contained in a fixed message if you can manage to get it out. A fall
back which just chucks out minimal information can also be useful (I
have a daemon that emails a fixed message if it crashes so that the
sysadmin at least knows it needs restarting before it tries to do any
tidying up or detailed error reporting).
--
Flash Gordon

"io_x" <> writes:
> "Keith Thompson" <kst-> ha scritto nel messaggio
> news:...
>> I agree that fclose() can do some checking, and that it shouldn't be
>> too difficult in most implementations. (I won't comment on your
>> characterization of an implementation that doesn't do this.)
>>
>> On the other hand, it's not hard to imagine an implementation that's
>> built on top of some lower level, where that lower level doesn't
>> provide this kind of checking.
>>
>> Also (somebody else mentioned this scenario), consider:
>>
>> fp1 = fopen(...);
>> fclose(fp1);
>
>> fp2 = fopen(...);
>> /* Suppose fp2 happens to point to the same FILE object that
>> fp1 had pointed to.
>> */
>> ...
>> fclose(fp1);
>> /* Oops, we closed fp1 twice, but the error isn't detected
>> because the pointer looks valid.
>> */
>
> in how i see the subject:
> here is the place where the program has to abort()

Then how you see it is inconsistent with the standard and, as far as I
can tell, with reality.

I agree that it would be nice if this particular error could be
detected (preferably by having fclose return an error indication
rather than by aborting the program), but I don't see how it could be
detected. I suppose the implementation could try to guarantee that
fopen() calls don't return a pointer to a previously closed file, but
that would mean remembering all previously closed files, which could
become unwieldy after a while. Remembering the last N, for some small
value of N, is probably feasible.

>> fclose(fp2);
>> /* This should have beeen ok, but the previous fclose clobbered
>> the FILE object; if fclose() performs checking, it will
>> report the error here rather than where it really occurred
>> */
>>
>> That's not to say that performing checking in fclose() isn't a good
>> idea (and closing a file isn't likely to be performance-critical), but
>> it can't catch everything.
>
> if the buffer of the file is returned from the malloc() function,
> "that performing checking" could be done in free() function too;
> free() could show there is a memory leak or better a double free

malloc and free can have the same kind of problem:

p1 = malloc(...);
free(p1);

p2 = malloc(...); /* p2 happens to point to the same location that
p1 previously pointed to */
free(p1); /* oops! */

The free(p1) call invokes undefined behavior, but without substantial
extra work there's no way the implementation can detect the problem.

--
Keith Thompson (The_Other_Keith) kst- <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

In article <>,
Keith Thompson <kst-> wrote:

>I agree that it would be nice if this particular error could be
>detected (preferably by having fclose return an error indication
>rather than by aborting the program), but I don't see how it could be
>detected. I suppose the implementation could try to guarantee that
>fopen() calls don't return a pointer to a previously closed file, but
>that would mean remembering all previously closed files, which could
>become unwieldy after a while.

It's not unreasonable in a debugging mode. Some malloc()
implementations have a debugging option that effectively makes free()
a no-op, so they never return the same memory twice. For a system
that malloc()s FILE structs, this would have the side-effect of doing
what you suggest.

-- Richard
--
Please remember to mention me / in tapes you leave behind.