Seg Fault on fopen/fclose

Question

I have a program that is creating multiple files. There is a function for each file being created. Within each function is the exact same code to create the file name, open/create the file for writing, set its permissions and at the end close the file. I decided to make a function for opening the file and closing the file so I can just call that instead of using the same code each time. The code previously looked like the following in each function:

void WriteFile1(char *name) {
   FILE *file;
   char *filename; //This is being malloc'ed because it initially consisted of multiple strings

   if (!(filename = malloc(sizeof(char *) * (strlen(name) + 1)))) MallocError();
   if (!(file = fopen(filename, "w"))) {
       fprintf(stderr, "Unable to open %s. Exiting \n", filename);
       exit(1);
   }
   fchmod(fileno(file), S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH);

   //a bunch of fprintf statements here

   if (fclose(file)) {
       fprintf(stderr, "Error closing %s. Exiting...\n", filename);
       exit(1);
   }
}

This worked perfectly fine. I had no issues. Now it looks like the following:

void WriteFile1() {
FILE *file;

OpenFile(file, "filename.asdf");
//fprintf statements
CloseFile(file, "filename.asdf");
}

void OpenFile(FILE *file, char *name) {
   if (!(file = fopen(name, "w"))) {
      fprintf(stderr, "Unable to open %s. Exiting... \n", name);
      exit(1);
   }
   fchmod(fileno(file), S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH);
}
void CloseFile(FILE *file, char *name) {
    if (fclose(file)) {
        fprintf(stderr, "Error closing %s. Exiting...\n", name);
        exit(1);
    }
}

It seg faults as soon as I get to the first fprintf statement in WriteFile1(). Am I doing something incorrectly with the FILE variable? It seems like it should work just like it did before. The only difference is the malloc of the filename string, which I am instead passing as name and giving the actual value in quotes.

Thank you

Answer 1

This piece of code is wrong:

void OpenFile(FILE *file, char *name) {
   if (!(file = fopen(name, "w"))) {

Here you are just assigning to the local file variable. You have to return the file, so your WriteFile1() function can work with that FILE*

void WriteFile1() {
FILE *file;

file = OpenFile("filename.asdf");
//fprintf statements
CloseFile(file, "filename.asdf");
}

FILE * OpenFile(char *name) {
   FILE * file;
   if (!(file = fopen(name, "w"))) {
      fprintf(stderr, "Unable to open %s. Exiting... \n", name);
      exit(1);
   }
   fchmod(fileno(file), S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH);
   return file;
}

Answer 2

Your open function should look like this:

FILE *  OpenFile( char *name) {
   FILE * file;
   if (!(file = fopen(name, "w"))) {
      fprintf(stderr, "Unable to open %s. Exiting... \n", name);
      exit(1);
   }
   fchmod(fileno(file), S_IRUSR | S_IWUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IROTH);
   return file;
}

In your version, the FILE * is effectively a local variable (as it is a parameter) of the function. Changing it in the function does not change it in the outside world.

When designing functions that return pointers (or anything else for that matter), always prefer to return the pointer via a return statement rather than trying to do it via the parameter list.

Answer 3

This:

filename = malloc(sizeof(char *) * (strlen(name) + 1))

should be:

filename = strdup(name);

if you have it, otherwise something like:

if((filename = malloc(strlen(name) + 1)) != NULL)
{
   strcpy(filename, name);
   ...
}

Note in particular that each character is just a char, not a char *. Since sizeof (char) == 1 is always true, there's no point in involving it at all.