If forget to fclose() after fopen(), is there any testing tool who can detect it automatically?

[lihua]

Hi, Group!
I got one question here:

We all know that fclose() must be called after file operations to
avoid unexpected errors.But there are really cases when you forget to
do that!Just
like what happens in memory operations, everyone knows the importance
of
freeing the allocated memory, but there do have memory leaks from time
to
time! For the memory leak case, there are lots of testing tools to
detect it
automatically like Rational Purify or other free ones.

So my question is: is there any testing tool right there which can
detect
forgetting-fclose defects automatically?

I have searched google pages and groups but seem to get no hint!

Any help will be greatly appreciated!

[websnarf]

lihua wrote:

We all know that fclose() must be called after file operations to
avoid unexpected errors.But there are really cases when you forget to
do that! Just like what happens in memory operations, everyone knows the
importance of freeing the allocated memory, but there do have memory leaks
from time to time! For the memory leak case, there are lots of testing tools
to detect it automatically like Rational Purify or other free ones.

So my question is: is there any testing tool right there which can
detect forgetting-fclose defects automatically?

In most C libraries, fopen() allocates memory for the FILE * structure
it returned expecting fclose() to deallocate it. In other words, the
best tool is something like Purify.

--
Paul Hsieh
http://www.pobox.com/~qed/
http://bstring.sf.net/

[MJ]

Hi
You can use dev partner .. it may be useful to find such kind of
problems
Mayur

[CBFalconer]

lihua wrote:

We all know that fclose() must be called after file operations to
avoid unexpected errors.But there are really cases when you forget
to do that!Just like what happens in memory operations, everyone
knows the importance of freeing the allocated memory, but there do
have memory leaks from time to time! For the memory leak case,
there are lots of testing tools to detect it automatically like
Rational Purify or other free ones.

So my question is: is there any testing tool right there which can
detect forgetting-fclose defects automatically?

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:

1. Wherever you declare the FILE* variable, initialize it to NULL.
2. Ensure that that scope has a single exit point.
3. At that exit, insert "if (f) fclose(f)", where f is the FILE*
variable.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

[Giorgos Keramidas]

CBFalconer <cb********@yahoo.com> writes:

lihua wrote:

We all know that fclose() must be called after file operations to
avoid unexpected errors.But there are really cases when you forget
to do that!
Be careful when you use FILE objects then :-)
Just like what happens in memory operations, everyone knows the
importance of freeing the allocated memory, but there do have memory
leaks from time to time! For the memory leak case, there are lots of
testing tools to detect it automatically like Rational Purify or
other free ones.

So my question is: is there any testing tool right there which can
detect forgetting-fclose defects automatically?

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:

1. Wherever you declare the FILE* variable, initialize it to NULL.

This is almost very good advice, but it's just a half-measure though and
hardly a solution to the real problem which is sloppiness when using
FILE objects. Initializing FILE pointers to NULL when they are
declared, i.e. with something like this:

1 #include <stdio.h>
2
3 int main(void)
4 {
5 FILE *fp = NULL;
6 int k;
7

will only serve as a warning the first time `fp' has to be accessed.

If the `fp' pointer is used in a loop and gets assigned to the result of
multiple fopen() calls, the second and all subsequent calls will not
have a NULL `fp' as an indication of something that is wrong with the
program:

8 for (k = 1; k < argc; k++) {
9 if (argv[k] == NULL)
10 continue;
11 fp = fopen(argv[k], "rb");
12
13 /* Do something with `fp' here. */
14
15 /* Missing fclose() call. */
16 }
17
18 return (EXIT_SUCCESS);
19 }

The only way to avoid leaking open FILE objects here is to be careful
when writing the program.

Instead of the original suggestion (Wherever you declare the FILE*
variable, initialize it to NULL), I usually prefer to "initialize
pointers to known values right before you start using them and use
assert() to watch out for 'strange' values in strategically chosen
places".

- Giorgos

[CBFalconer]

Giorgos Keramidas wrote:

CBFalconer <cb********@yahoo.com> writes:

lihua wrote:

We all know that fclose() must be called after file operations to
avoid unexpected errors.But there are really cases when you forget
to do that!
Be careful when you use FILE objects then :-)
Just like what happens in memory operations, everyone knows the
importance of freeing the allocated memory, but there do have memory
leaks from time to time! For the memory leak case, there are lots of
testing tools to detect it automatically like Rational Purify or
other free ones.

So my question is: is there any testing tool right there which can
detect forgetting-fclose defects automatically?

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:

1. Wherever you declare the FILE* variable, initialize it to NULL.

This is almost very good advice, but it's just a half-measure though and
hardly a solution to the real problem which is sloppiness when using
FILE objects. Initializing FILE pointers to NULL when they are
declared, i.e. with something like this:

You miss the point, and snipped the example. Initializing to NULL
at declaration means you can tell whether it was used at scope
exit, and thus can safely call fclose on it.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

[Giorgos Keramidas]

CBFalconer <cb********@yahoo.com> writes:

Giorgos Keramidas wrote:

CBFalconer <cb********@yahoo.com> writes:

lihua wrote:

We all know that fclose() must be called after file operations to
avoid unexpected errors.But there are really cases when you forget
to do that!

Be careful when you use FILE objects then :-)

Just like what happens in memory operations, everyone knows the
importance of freeing the allocated memory, but there do have memory
leaks from time to time! For the memory leak case, there are lots of
testing tools to detect it automatically like Rational Purify or
other free ones.

So my question is: is there any testing tool right there which can
detect forgetting-fclose defects automatically?

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:
>
> 1. Wherever you declare the FILE* variable, initialize it to NULL.

This is almost very good advice, but it's just a half-measure though and
hardly a solution to the real problem which is sloppiness when using
FILE objects. Initializing FILE pointers to NULL when they are
declared, i.e. with something like this:

You miss the point, and snipped the example. Initializing to NULL
at declaration means you can tell whether it was used at scope
exit, and thus can safely call fclose on it.

I wasn't clear enough it seems. Checking at scope exit breaks
immediately the moment you start calling fopen() inside loop constructs.

You *are* right that checking at scope exit is good. I'm not arguing
against that. I'm merely pointing out that it's not a panacea.

Regards,
Giorgos

[Mark]

"CBFalconer" <cb********@yahoo.com> wrote in message
news:42***************@yahoo.com...

Giorgos Keramidas wrote:

CBFalconer <cb********@yahoo.com> writes:

lihua wrote:

We all know that fclose() must be called after file operations to
avoid unexpected errors.But there are really cases when you forget
to do that!
Be careful when you use FILE objects then :-)

Just like what happens in memory operations, everyone knows the
importance of freeing the allocated memory, but there do have memory
leaks from time to time! For the memory leak case, there are lots of
testing tools to detect it automatically like Rational Purify or
other free ones.

So my question is: is there any testing tool right there which can
detect forgetting-fclose defects automatically?

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:
>
> 1. Wherever you declare the FILE* variable, initialize it to NULL.

This is almost very good advice, but it's just a half-measure though and
hardly a solution to the real problem which is sloppiness when using
FILE objects. Initializing FILE pointers to NULL when they are
declared, i.e. with something like this:

You miss the point, and snipped the example.

You didn't provide an example, and then snipped his!
Initializing to NULL
at declaration means you can tell whether it was used at scope
exit, and thus can safely call fclose on it.

Not necessarily... fclose may have already been called (pointer not reset)

consider
void func(void)
{
FILE *fp = NULL;
for(;;) {
...
if((fp = fopen(file, "r")) != NULL) {
...
fclose(fp);
}
...
break;
}
if(fp) // will probably be set
fclose(fp); // shouldn't be called
return;
}
Regards,
Mark

[CBFalconer]

Giorgos Keramidas wrote:

CBFalconer <cb********@yahoo.com> writes:

Giorgos Keramidas wrote:

CBFalconer <cb********@yahoo.com> writes:
lihua wrote:
> .... snip ...>
> So my question is: is there any testing tool right there which
> can detect forgetting-fclose defects automatically?

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:

1. Wherever you declare the FILE* variable, initialize it to NULL.
*** Restored snippage so things make sense to others ***
2. Ensure that that scope has a single exit point.
3. At that exit, insert "if (f) fclose(f)", where f is the
FILE* variable.
*** End of restored snippage ***

This is almost very good advice, but it's just a half-measure though
and hardly a solution to the real problem which is sloppiness when
using FILE objects. Initializing FILE pointers to NULL when they
are declared, i.e. with something like this:

You miss the point, and snipped the example. Initializing to NULL
at declaration means you can tell whether it was used at scope
exit, and thus can safely call fclose on it.

I wasn't clear enough it seems. Checking at scope exit breaks
immediately the moment you start calling fopen() inside loop
constructs.

You *are* right that checking at scope exit is good. I'm not
arguing against that. I'm merely pointing out that it's not a
panacea.

You still miss the point. Any successful fopens, within loops or
elsewhere, will leave the pointer non-NULL, so the scope exit code
can tell that a fclose is needed. Any successful earlier fcloses
within that scope need to also set the pointer to NULL. The state
of the pointer, NULL or non-NULL, signals whether it refers to an
open file. This way the scope will _never_ exit leaving unclosed
abandoned files behind, which was the OPs objective.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

[Giorgos Keramidas]

CBFalconer <cb********@yahoo.com> writes:

CBFalconer <cb********@yahoo.com> writes:

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:

1. Wherever you declare the FILE* variable, initialize it to NULL.
*** Restored snippage so things make sense to others ***
2. Ensure that that scope has a single exit point.
3. At that exit, insert "if (f) fclose(f)", where f is the
FILE* variable.
*** End of restored snippage ***

CBFalconer <cb********@yahoo.com> writes: Giorgos Keramidas writes:

CBFalconer <cb********@yahoo.com> writes:

Giorgos Keramidas writes:
> This is almost very good advice, but it's just a half-measure though
> and hardly a solution to the real problem which is sloppiness when
> using FILE objects. Initializing FILE pointers to NULL when they
> are declared, i.e. with something like this:

You miss the point, and snipped the example. Initializing to NULL
at declaration means you can tell whether it was used at scope
exit, and thus can safely call fclose on it.

I wasn't clear enough it seems. Checking at scope exit breaks
immediately the moment you start calling fopen() inside loop
constructs.

You *are* right that checking at scope exit is good. I'm not
arguing against that. I'm merely pointing out that it's not a
panacea.

You still miss the point. Any successful fopens, within loops or
elsewhere, will leave the pointer non-NULL, so the scope exit code
can tell that a fclose is needed. Any successful earlier fcloses
within that scope need to also set the pointer to NULL. The state
of the pointer, NULL or non-NULL, signals whether it refers to an
open file. This way the scope will _never_ exit leaving unclosed
abandoned files behind, which was the OPs objective.

Of course the "scope may exit", depending on the definition of "scope"
you use and the placement of the (FILE *) object's declaration, i.e. the
"scope" of a (FILE *) is not necessarily the loop construct, unless you
are referring to declarations like:

while (1) {
FILE *fp = NULL;

if ((fp == fopen(...) == NULL)
break;

/* Some code that uses `fp'. */
if (fp != NULL)
fclose(fp);
}

The same is not true though for declarations like the one I presented in
a previous post:

{
FILE *fp = NULL;

while (1) {
if ((fp = fopen(...)) == NULL)
break;

/* Do something with fp. */
fclose(fp);
}
}

In this second case, it's the responsibility of the programmer to make
sure tha every fopen() has a matching fclose(). The same responsibility
falls on the shoulders of the programmer when the fopen() and fclose()
call are wrapped around higher level interfaces, like for example:

FILE *libfoo_openlog(const char *filename);
void libfoo_closelog(FILE *fp);

Anyway, this already feels like a rather pointless argument, so I'm not
going to post anything else in this thread.

- Giorgos

[CBFalconer]

Giorgos Keramidas wrote:

.... snip ...
The same is not true though for declarations like the one I
presented in a previous post:

{
FILE *fp = NULL;

while (1) {
if ((fp = fopen(...)) == NULL)
break;
/* Do something with fp. */
fclose(fp);
}
}

In this second case, it's the responsibility of the programmer to
make sure tha every fopen() has a matching fclose(). The same
responsibility falls on the shoulders of the programmer when the
fopen() and fclose() call are wrapped around higher level
interfaces, like for example:

In this case you haven't followed the recipe I gave before, which
would require an "if (fp) fclose(fp);" between the ultimate and
penultimate '}'s. All you have to do is watch the scope of the
fp. You also neglected to set it to NULL after the interior
close. You can automate this latter by a routine:

int fclosenul(FILE* *fp) {
int ans;
if (EOF != (ans = fclose(*fp)}) *fp = NULL;
return ans;
}

By doing those things you make the rest of the coding much less
sensitive to minor mistakes.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

[CBFalconer]

Mark wrote:

"CBFalconer" <cb********@yahoo.com> wrote in message

Giorgos Keramidas wrote:

CBFalconer <cb********@yahoo.com> writes:
lihua wrote:
>
> We all know that fclose() must be called after file operations to
> avoid unexpected errors.But there are really cases when you forget
> to do that!

Be careful when you use FILE objects then :-)

> Just like what happens in memory operations, everyone knows the
> importance of freeing the allocated memory, but there do have memory
> leaks from time to time! For the memory leak case, there are lots of
> testing tools to detect it automatically like Rational Purify or
> other free ones.
>
> So my question is: is there any testing tool right there which can
> detect forgetting-fclose defects automatically?

If you keep track of what you are doing there should be no
problem. However if you need to protect against your own
sloppiness, try this:

1. Wherever you declare the FILE* variable, initialize it to
NULL.

This is almost very good advice, but it's just a half-measure
though and hardly a solution to the real problem which is
sloppiness when using FILE objects. Initializing FILE pointers
to NULL when they are declared, i.e. with something like this:

You miss the point, and snipped the example.

You didn't provide an example, and then snipped his!

Initializing to NULL
at declaration means you can tell whether it was used at scope
exit, and thus can safely call fclose on it.

Not necessarily... fclose may have already been called (pointer
not reset)

consider
void func(void)
{
FILE *fp = NULL;
for(;;) {
...
if((fp = fopen(file, "r")) != NULL) {
...
fclose(fp);
}
...
break;
}
if(fp) // will probably be set
fclose(fp); // shouldn't be called
return;
}

Change the interior fclose(fp) to:

if (EOF != fclose(fp)) fp = NULL;

and all will be well as far as file handling goes. Think about
loop invariants. This one is "((fp == NULL) ^ (fp is open)) ==
1". By maintaining that we simply have to test fp == NULL.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

[Villy Kruse]

On Wed, 06 Jul 2005 04:07:27 GMT,
CBFalconer <cb********@yahoo.com> wrote:

Change the interior fclose(fp) to:

if (EOF != fclose(fp)) fp = NULL;

and all will be well as far as file handling goes. Think about
loop invariants. This one is "((fp == NULL) ^ (fp is open)) ==
1". By maintaining that we simply have to test fp == NULL.

If fclose(fp) returns EOF is fp then still open? My man page, which
isn't normative for ANSI C, says no further access to fp is alowed
whether the fclose function returns an error or not. If no further
access to fp is allowed then it should unconditionaly be set to NULL.

Villy

[Rajan]

I have modified the code just by one statement, wherein you initalize
everytime.

for (k = 1; k < argc; k++) {
FILE* fp = NULL; // Added by Rajendra S.
9 if (argv[k] == NULL)
10 continue;
11 fp = fopen(argv[k], "rb");
12
13 /* Do something with `fp' here. */
14
15 /* Missing fclose() call. */
16 }
17
18 return (EXIT_SUCCESS);
19 }

[Giorgos Keramidas]

CBFalconer <cb********@yahoo.com> writes:

Giorgos Keramidas wrote:

... snip ...

Thanks a lot for snipping the one example that shows clearly that a
single "if (fp) fclose(fp);" after a loop fails to work as a prevention
or fix of the "missing fclose()" problem.

Not appreciated at all.

The same is not true though for declarations like the one I
presented in a previous post:

{
FILE *fp = NULL;

while (1) {
if ((fp = fopen(...)) == NULL)
break;
/* Do something with fp. */
fclose(fp);
}
}

In this second case, it's the responsibility of the programmer to
make sure tha every fopen() has a matching fclose(). The same
responsibility falls on the shoulders of the programmer when the
fopen() and fclose() call are wrapped around higher level
interfaces, like for example:

In this case you haven't followed the recipe I gave before, which
would require an "if (fp) fclose(fp);" between the ultimate and
penultimate '}'s. All you have to do is watch the scope of the
fp. You also neglected to set it to NULL after the interior
close.

You have a valid point, which happens to be identical to mine, i.e. "be
vigilant and very careful to match file 'open' calls with their
respective 'close' calls". I just don't think a "recipe" like:

Add "if (fp) fclose(fp);" at the end of scope X

will solve all potential problems and/or help detecting them.

It won't. No matter how many times you redefine "scope" and how many
tricks you play by moving the "if (fp) fclose(fp);" trick around.

All I'm saying is not that this might work or it might not. It takes a
lot more care and attention to what's going on and there is no easy way
to check for missing fclose() bugs, except through indirect means
(i.e. because memory of (FILE) objects is leaked at program exit).

- Giorgos

[Giorgos Keramidas]

"Rajan" <rs*******@yahoo.com> writes:

I have modified the code just by one statement, wherein you initalize
everytime.

for (k = 1; k < argc; k++) {
FILE* fp = NULL; // Added by Rajendra S.
9 if (argv[k] == NULL)
10 continue;
11 fp = fopen(argv[k], "rb");
12
13 /* Do something with `fp' here. */
14
15 /* Missing fclose() call. */
16 }
17
18 return (EXIT_SUCCESS);
19 }

You still leak FILE objects and this fails to build with C90 compilers,
since declarations are not allowed inside the while() block. The
initialization to NULL is also pretty redundant, since you just go ahead
and overwrite it with the return value of fopen() a few lines below.

Being ultra careful about initializing `fp' to a well known value is ok,
but it requires explicit action by the programmer. Hence it's not a
"testing tool" and it's certainly not automagic in any way.

There's only one place where an fclose() call would be valid above, and
that is at line 15. If the programmer is careful enough to properly
initialize `fp' before using it though, there is no reason to use "if
(fp != NULL)". An assert() is much better, IMO.

FILE *fp;

for (k = 1; k < argc; k++) {
if (argv[k] == NULL)
continue;
fp = fopen(argv[k], "rb");

/* Use "fp" here. */

assert(fp != NULL);
fclose(fp);
fp = NULL;
}

Call me stubborn, but I still stand by my original position that there
is no magic wand that can work as a panacea of all possible fclose()
leaks.

- Giorgos

[Christian Kandeler]

Giorgos Keramidas wrote:

for (k = 1; k < argc; k++) {
FILE* fp = NULL; // Added by Rajendra S.
9 if (argv[k] == NULL)
10 continue;
11 fp = fopen(argv[k], "rb");
12
13 /* Do something with `fp' here. */
14
15 /* Missing fclose() call. */
16 }
17
18 return (EXIT_SUCCESS);
19 }

You still leak FILE objects and this fails to build with C90 compilers,
since declarations are not allowed inside the while() block.

Your second claim is wrong.
Christian

[CBFalconer]

Villy Kruse wrote:

CBFalconer <cb********@yahoo.com> wrote:

Change the interior fclose(fp) to:

if (EOF != fclose(fp)) fp = NULL;

and all will be well as far as file handling goes. Think about
loop invariants. This one is "((fp == NULL) ^ (fp is open)) ==
1". By maintaining that we simply have to test fp == NULL.

If fclose(fp) returns EOF is fp then still open? My man page, which
isn't normative for ANSI C, says no further access to fp is alowed
whether the fclose function returns an error or not. If no further
access to fp is allowed then it should unconditionaly be set to NULL.

The standard, or at least N869, is silent on that:

7.19.5.1 The fclose function

Synopsis

[#1]
#include <stdio.h>
int fclose(FILE *stream);

Description

[#2] The fclose function causes the stream pointed to by
stream to be flushed and the associated file to be closed.
Any unwritten buffered data for the stream are delivered to
the host environment to be written to the file; any unread
buffered data are discarded. The stream is disassociated
from the file. If the associated buffer was automatically
allocated, it is deallocated.

Returns

[#3] The fclose function returns zero if the stream was
successfully closed, or EOF if any errors were detected.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

[Joe Wright]

CBFalconer wrote:

Villy Kruse wrote:

CBFalconer <cb********@yahoo.com> wrote:

Change the interior fclose(fp) to:

if (EOF != fclose(fp)) fp = NULL;

and all will be well as far as file handling goes. Think about
loop invariants. This one is "((fp == NULL) ^ (fp is open)) ==
1". By maintaining that we simply have to test fp == NULL.

If fclose(fp) returns EOF is fp then still open? My man page, which
isn't normative for ANSI C, says no further access to fp is alowed
whether the fclose function returns an error or not. If no further
access to fp is allowed then it should unconditionaly be set to NULL.

The standard, or at least N869, is silent on that:

7.19.5.1 The fclose function

Synopsis

[#1]
#include <stdio.h>
int fclose(FILE *stream);

Description

[#2] The fclose function causes the stream pointed to by
stream to be flushed and the associated file to be closed.
Any unwritten buffered data for the stream are delivered to
the host environment to be written to the file; any unread
buffered data are discarded. The stream is disassociated
from the file. If the associated buffer was automatically
allocated, it is deallocated.

Returns

[#3] The fclose function returns zero if the stream was
successfully closed, or EOF if any errors were detected.

I always check fopen() for success because if not, I can't use the file.
Likewise, I check malloc() for success in order to use the memory. If
either of these fail, I tell the user somehow and quit because my
program can't work.

But fopen() was successful and I have spent all day reading and/or
writing to this file. Now the program is over and I fclose() the file.
And it fails (EOF). What shall I do now? Tell the user and quit.

I am very careful to fclose() each file I have open but I can't remember
ever checking the value of fclose(). It might as well return void as
free() does because there is virtually nothing you can do in case of
error anyway.

This is not a troll, but a lure. If I'm wrong I'd like to know it.

--
Joe Wright
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---

[CBFalconer]

Joe Wright wrote:

.... snip ...
I am very careful to fclose() each file I have open but I can't
remember ever checking the value of fclose(). It might as well
return void as free() does because there is virtually nothing
you can do in case of error anyway.

Yes there is. You can decide not to destroy the input files that
you used to create that output. You can decide to moan to some
passing human. You might even decide to try again, but that result
is likely to be iffy.

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson