fclose... what is better

[collinm]

hi

is it better to do:

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
fclose(fp);
}
else
printf("fail\n" );
or

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);

thanks

[Walter Roberson]

In article <11************ **********@z14g 2000cwz.googleg roups.com>,
collinm <co*****@laboit eaprog.com> wrote:

is it better to do:
or FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);

What is the defined behaviour of fclose(NULL) ?

If you don't know off the top of your head, chances are that
other people maintaining your program will not either. Thus,
regardless of the defined behaviour of fclose(NULL), your
program would be more maintenance-friendly if you do not
invoke that behaviour.
By the way, as a minor matter of style: messages such as "fail"
are most often associated with errors rather than with normal
program behaviour. Messages associated with errors are usually
sent to stderr instead of stdout.

If the unavailability of the file is a routine matter for the program,
then the associated message would usually not be "fail" but instead
something closer to "skipping unavailable file %s\n" or "Sorry, no
winking blinking lights available today!\n"
--
Studies show that the average reader ignores 106% of all statistics
they see in .signatures.

[Thomas Matthews]

collinm wrote:

hi

is it better to do:

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
fclose(fp);
}
else
printf("fail\n" );
or

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);

thanks

All depends on your style and error handling.
My preference is:
unsigned char successful = 0;
fp = fopen(...);
successful = fp == NULL;
if (!successful)
{
/* File open failure */
}

if (successful)
{
successful = Process_File_Co ntents();
}

if (successful)
{
/* ... */
}

As for when to close a file, my opinion is to
close it as soon as you are finished with it.
Leaving a file open for longer than necessary
may allow things to happen to the internal data
{by your program or other ones).

--
Thomas Matthews

C++ newsgroup welcome message:
http://www.slack.net/~shiva/welcome.txt
C++ Faq: http://www.parashift.com/c++-faq-lite
C Faq: http://www.eskimo.com/~scs/c-faq/top.html
alt.comp.lang.l earn.c-c++ faq:
http://www.comeaucomputing.com/learn/faq/
Other sites:
http://www.josuttis.com -- C++ STL Library book
http://www.sgi.com/tech/stl -- Standard Template Library

[Fred L. Kleinschmidt]

collinm wrote:

hi

is it better to do:

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
fclose(fp);
}
else
printf("fail\n" );

or

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);
Do not call fclose(fp) if fp is NULL.

thanks

--
Fred L. Kleinschmidt
Boeing Associate Technical Fellow
Technical Architect, Common User Interface Services
M/S 2R-94 [phone]

[Eric Sosman]

collinm wrote:

hi

is it better to do:

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
fclose(fp);
}
else
printf("fail\n" );
or

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);

"Better" depends on what you want to accomplish, of
course. However, note that if the fopen() fails in the
second version you will wind up calling fclose(NULL),
which is Not A Good Idea ...

--
Er*********@sun .com

[Malcolm]

"collinm" <co*****@laboit eaprog.com> wrote in message

is it better to do:

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
fclose(fp);
}
else
printf("fail\n" );
or

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);

As others ahve pointed out, you don't want to call fclose() on a null
pointer.

The fopen() expression is not a disaster, but makes it look as if the
purpose of the call is to check a condition, whilst in fact the purpose is
to assign a file pointer to fp.

So

FILE *fp;

fp = fopen(local_led , "r");
if(fp)
{
/* manipulate the file */
fclose(fp);
}
else
printf("fail\n" );

is the style that I use.

[August Karlstrom]

collinm wrote:

hi

is it better to do:

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
fclose(fp);
}
else
printf("fail\n" );
or

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);

thanks

I think the general recommendation is to deal with exceptions first:

FILE *fp;

fp = fopen(local_led , "r");
if(fp == NULL) {
fprintf(stderr, "Could not open the file %s", local_red);
exit(EXIT_FAILU RE);
}
....
fclose(fp);

-- August

[Mark]

"August Karlstrom" <fu********@com hem.se> wrote in message
news:qX******** *************@n ewsc.telia.net. ..

collinm wrote:

hi

is it better to do:

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
fclose(fp);
}
else
printf("fail\n" );
or

FILE *fp;
if((fp = fopen(local_led , "r"))!=NULL )
{
printf("oepn\n" );
}
else
printf("fail\n" );
fclose(fp);

thanks

I think the general recommendation is to deal with exceptions first:

FILE *fp;

fp = fopen(local_led , "r");
if(fp == NULL) {
fprintf(stderr, "Could not open the file %s", local_red);
exit(EXIT_FAILU RE);
}
...
fclose(fp);

-- August

'THE' general recommendation? BS.
What happens when you can't exit() because of your requirements?
Do you replace the call with a return? I've fixed more problems caused
by sloppy coding conventions similar to this one you 'recommend'...
especially when multiple files are being opened and manipulated at once.
My personal recommendation would be to code an fopen() as follows:

FILE *fp;
if((fp = fopen(local_led , "r"))) {

fclose(fp);
} else
perror(local_le d);

Mark

[August Karlstrom]

Mark wrote:

"August Karlstrom" <fu********@com hem.se> wrote in message

I think the general recommendation is to deal with exceptions first:

FILE *fp;

fp = fopen(local_led , "r");
if(fp == NULL) {
fprintf(stderr, "Could not open the file %s", local_red);
exit(EXIT_FAILU RE);
}
...
fclose(fp);

-- August

'THE' general recommendation? BS.
What happens when you can't exit() because of your requirements?
Do you replace the call with a return? I've fixed more problems caused
by sloppy coding conventions similar to this one you 'recommend'...
especially when multiple files are being opened and manipulated at once.
My personal recommendation would be to code an fopen() as follows:

FILE *fp;
if((fp = fopen(local_led , "r"))) {

fclose(fp);
} else
perror(local_le d);

Mark

Okay, my point was really the order of the branches:

FILE *fp;

fp = fopen(local_led , "r");
if(fp == NULL) {
/* Do whatever has to be done when local_red cannot be opened. */
} else {
...
fclose(fp);
}

Of course, what you have do when `fopen' fails depends on how the
program is structured; preconditions, postconditions etc. (My previous
example didn't need the else clause because of the invokation of `exit'.)

-- August

[Dave Thompson]

On Thu, 19 May 2005 13:56:37 GMT, Thomas Matthews
<Th************ *************@s bcglobal.net> wrote:
<snip>

All depends on your style and error handling.
My preference is:
unsigned char successful = 0;
fp = fopen(...);
successful = fp == NULL;
Shirley <G> you mean fp != NULL .
if (!successful)
{
/* File open failure */
}

if (successful)
{
successful = Process_File_Co ntents();
You should pass fp, otherwise it needs to be a shared "global" (at
least file-scope static, maybe more) which is yucky(tm) far beyond
this mildly clunky error handling.

Presumably the fclose() is done within Process_File_Co ntents(), in
which case the name is not fully descriptive, or added here ...
}
because now 'successful' no longer tells you accurately whether the
fopen was successful -- although fp does, it you are willing to
violate the structuring you just put so much effort into.
if (successful)
{
/* ... */
}

As for when to close a file, my opinion is to
close it as soon as you are finished with it.
Leaving a file open for longer than necessary
may allow things to happen to the internal data
{by your program or other ones).

Promptly closing a file (not opened read-only enforced by the OS) can
protect it from malfunctions in other parts of the same program --
although it would be better to fix those malfunctions anyway -- but is
very unlikely to help protect against other _programs_; indeed it may
release locks and allow _more_ access to the file by other programs.

- David.Thompson1 at worldnet.att.ne t