By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
448,837 Members | 1,648 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 448,837 IT Pros & Developers. It's quick & easy.

fclose... what is better

P: n/a
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

Nov 14 '05 #1
Share this Question
Share on Google+
10 Replies


P: n/a
In article <11**********************@z14g2000cwz.googlegroups .com>,
collinm <co*****@laboiteaprog.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.
Nov 14 '05 #2

P: n/a
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_Contents();
}

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.learn.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
Nov 14 '05 #3

P: n/a


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 (206)544-5225
Nov 14 '05 #4

P: n/a


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

Nov 14 '05 #5

P: n/a

"collinm" <co*****@laboiteaprog.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.
Nov 14 '05 #6

P: n/a
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_FAILURE);
}
....
fclose(fp);

-- August
Nov 14 '05 #7

P: n/a
"August Karlstrom" <fu********@comhem.se> wrote in message
news:qX*********************@newsc.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_FAILURE);
}
...
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_led);

Mark
Nov 14 '05 #8

P: n/a
Mark wrote:
"August Karlstrom" <fu********@comhem.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_FAILURE);
}
...
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_led);

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
Nov 14 '05 #9

P: n/a
On Thu, 19 May 2005 13:56:37 GMT, Thomas Matthews
<Th*************************@sbcglobal.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_Contents();
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_Contents(), 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.net
Nov 14 '05 #10

P: n/a
On Tue, 31 May 2005 03:49:09 GMT, Dave Thompson
<da*************@worldnet.att.net> wrote:
On Thu, 19 May 2005 13:56:37 GMT, Thomas Matthews
<Th*************************@sbcglobal.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 .


I always put parentheses round conditions in assignments, it's too easy
to read (and type and not notice) it as

successful = fp = NULL;

but putting

successful = (fp == NULL);

makes the condition more obvious, and thus the logical error you point
out stands out more as well.

cHris C
Nov 14 '05 #11

This discussion thread is closed

Replies have been disabled for this discussion.