File pointer behaviour ?

Question

This is with reference to the question I asked earlier -

What is the correct way to declare and use a FILE * pointer in C/C++?

MyFile.h
char         sMsg[712]      = "";

#define STD_MSG(string) \
fprintf (stderr, string)

#define ERR_MSG(fp, string) \
fprintf (fp, "%s\n", string);\
fflush (fp)

MyFile.C
#include "PdmTestClnt.h"

//---------------------------------------------------------------
// ** Global variables
//---------------------------------------------------------------
FILE * fpErr = NULL;

funcxyz() {
//FILE * fpErr1 = NULL; 
sprintf (sMsg, "************ CHECKING FOR THE CRASH ************. \n");
ERR_MSG (fpErr, sMsg);
//ERR_MSG (fpErr1, sMsg);
}

//========================================================================
// Main
//========================================================================
integer main (integer argc, char ** argv)
{
   //FILE * fpErr = NULL;

   if (!(fpErr = sysFopen (sErrFileName, "a+")))
   {
      sprintf (sMsg,"Error in opening file %s", sErrFileName);
      STD_MSG (sMsg);
   }

     // Log in the error file
     sprintf (sMsg, "Log into the error file. \n");
     ERR_MSG (fpErr, sMsg);

funcxyz();

}

If the File pointer is declared global it works. But if it is declared local it results in Memory fault(coredump).

Ran on:

HP Unix Itanium

aCC compiler (C++ Compiler)

Can somebody explain the behaviour?

Edit: Sorry for not editing the question. I now understand the problem with printf()/fprintf(). I showed the results for printf()/fprintf() with Dev C++ compiler on Windows in my answer. But when I run it on HP Unix with aCC compiler it ignores %s completely and always prints the string correctly. So how do I ask my architect to change it without showing him it memory fault on Unix?

Thanks.

Answer 1

This information is irrelevent to the question. I should read more carefully before I answer. =X

Your problem is that you're shadowing the fpErr on the global scope with the one in the local scope. For example:

int var = 0;

void print_var() {
    printf("print_var: %d\n", var);
}

int main() {
    int var = 42;

    printf("main: %d\n", var);
    print_var();

    return 0;
}

If you run the code, the output should be:

main: 42
print_var: 0

In your case, fpErr has a value of NULL (0) and thus the file I/O functions try accessing data at NULL, which causes the segmentation fault.

Answer 2

Assuming by local you mean in funcxyz() uncommenting fpErr1, it segfaults because you don't open a file. You can't just use NULL there, where would you expect the data to go?

I would suggest writing funcxyz to take fpErr1 as a parameter, e.g.:

funcxyz(FILE *fpErr1) {
   //FILE * fpErr1 = NULL; 
   sprintf (sMsg, "************ CHECKING FOR THE CRASH ************. \n");
   ERR_MSG (fpErr1, sMsg);
   //ERR_MSG (fpErr1, sMsg);
}

And then calling it from main like:

 ...
 funcxyz(fpErr);
 ...

Answer 3

Don't define variables in headers - it is a nasty habit to get into.

Remove the initializer from sMsg[] in the header, and prepend 'extern' - and do define the variable, with initializer, in an appropriate source file (usually MyFile.c if the header is MyFile.h). This really matters when MyFile.h is used by several source files - and if it is used by just one source file, why were you using a header in the first place?

Your code also includes 'PdmTestClnt.h' and not MyFile.h - should we assume that MyFile.h is what you meant to include?

funcxyz() has no return type - it won't compile in C++ or under a strict C99 compiler. Why does the function format into sMsg, and then use fprintf() to copy the string? fprintf() can do the whole job (and then some).

Why you need a global definition

When you have a global variable, the code in main() initializes it by calling fopen(), and the other functions can use the initialized value. That's convenient. When you have a local variable, you have to initialize it. That's a pain because you'd end up opening the file many times which has numerous unwanted side effects - too many file handles in use, you have to close them too, and you probably keep truncating the output already in the file. To avoid that, pass the file pointer to the functions - or accept that a global is OK. Think about it - in some shape or form, the names stdin, stdout and stderr refer to global variables too.

void funcxyz(FILE *fp)
{
    sprintf(sMsg, "************ CHECKING FOR THE CRASH ************. \n");
    ERR_MSG (fpErr, sMsg);
}

int main(int argc, char **argv)
{
    FILE *fpErr = NULL;

    if ((fpErr = sysFopen(sErrFileName, "a+")) != 0)
    {
        sprintf(sMsg,"Error in opening file %s", sErrFileName);
        STD_MSG(sMsg);
    }

    // Log in the error file
    sprintf(sMsg, "Log into the error file. \n");
    ERR_MSG(fpErr, sMsg);

    funcxyz(fpErr);
    return(0);
}

Answer 4

Thanks for the answers. It was a very basic concept that every C or C++ programmer should know. And sorry for the #include, it was a copy/paste mistake.

I saw one comment on STD_MSG macro. Can someone explain a bit more as to why it is dangerous?

Answer 5

Thanks Dan, for explaining fprintf(). I tried it with Dev C++ compiler on Windows Vista and got the following results:

printf("Hello %s World"); -----> Sometimes Memory Fault and sometimes prints junk.

fprintf(stderr, "Hello %s World"); -----> Hello ♦↨╦ World

So fprintf() never memory faults. How?