Segmentation fault 11 printing string from struct

Question

This is the first time I've run into Segmentation fault 11 in C and I can't seem to wrap my head around what is actually going wrong.

What I'm trying to do is write a few int values to a struct plus the file name from the command line (char *) from a child process and then write the struct to the pipe to read from from the parent process. It works fine when it's only the integers and I take out the code working with the string, but once I add in the string and try to print out the file name in the parent process I get the segmentation fault 11 when the program is run.

I've looked at various posts from all over, but have noticed that the common issue for this is when someone attempts to assign a string to a char array and prints, but I made sure to use only char * here. Here's the code where it locks up

 if((read(pd[0], &pv, 2048)) == -1)
 {
   error_exit("read not working");
 }

 printf("words = %d\n", pv.words);
 printf("lines = %d\n", pv.lines);
 printf("bytes = %d\n", pv.bytes);
 printf("file = %s\n", pv.file); //locks up here and gives segmentation fault 11 on the command line

Here is the read out of what the program does when I run it:

$ ./a testfile
Parent process... should be waiting on child...
In child process! pid = 21993  
it worked? testfile
Done with child process!
words = 1
lines = 2
bytes = 3
Segmentation fault: 11

Also here is the full code EDIT: I swapped out the code using sizeof for string and used strlen

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>

void error_exit(char *);

typedef struct total {

  int words, lines, bytes;
  char *file;

} Vals;

int main(int argc, char *argv[])
{

  int pd[2]; //pipe descriptor
  pid_t pid;
  Vals v, pv;
  char *fname = "Not set";

  if(argc > 1)
  {
    fname = malloc(strlen(argv[1]));
    strcpy(fname, argv[1]);
  }

  if((pipe(pd)) == -1)
  {
    error_exit("pipe creation");
  }

  if((pid = fork()) == -1)
  { 
    error_exit("the fork forked up!");
  }
  else if(pid == 0)
  {

    printf("In child process! pid = %d\n", getpid());
    v.words = 1;
    v.lines = 2;
    v.bytes = 3;
    v.file = malloc(strlen(fname));
    strcpy(v.file, fname);
    printf("it worked? %s\n", v.file);

    close(pd[0]);

    if((write(pd[1], &v, sizeof(v.words) + sizeof(v.lines) + sizeof(v.bytes) + strlen(v.file))) == -1)
    {
      error_exit("Write from child");
    }

    //return; //return from child
    printf("Done with child process!\n");
    close(pd[1]);
    return 0;
  }
  else
  {
    printf("Parent process... should be waiting on child...\n");
  }
  //wait for child
  while((pid = wait(NULL)) > 0);

  close(pd[1]);

  //Vals pv = {0, 0, 0, "pv.file not set"};

  //just assign anything to file to see if it fixes
  //pv.file = malloc(strlen(fname));

  if((read(pd[0], &pv, 2048)) == -1)
  {
    error_exit("read not working");
  }

  printf("words = %d\n", pv.words);
  printf("lines = %d\n", pv.lines);
  printf("bytes = %d\n", pv.bytes);
  printf("file = %s\n", pv.file); //locks up here and gives segmentation fault 11 on the command line

  close(pd[0]);

  //program ended normally
  return 0;

}

void error_exit(char *err)
{
  printf("exiting because of this section: %s\nerrno = %d", err, errno);
  exit(1);
}

I really appreciate any insight on this!

I think that you want strlen(v.file) instead of sizeof(v.file). — fvu
Oh yeah, that makes life a lot easier :) I changed that throughout the code, but still get the segmentation fault. I did notice that before the parent reading from the pipe if I do pv.file = malloc(strlen(fname)); then it won't give me the segmentation fault but it prints out an empty string. — Frank A.
also, sizeof(argv[1]), same problem. And fname = argv[1]; probably also doesn't do what you want it to do. Try strcpy. — fvu
don't send pointers over a pipe; they are useless on other virtual address spaces — Diego
Also, to make debugging easier, I would save the return value from write() and read () (they contain the number of bytes actually written and read) and print them out, — fvu

kaylum kaylum · Accepted Answer · 2015-05-07T00:39:56

Your main problem is that you don't quite have the right understanding of C strings. You cannot do sizeof(char_pointer). That will just give you the pointer size (4 in a 32 bit system) and not the size of the string it points to. Use strlen to get the length of a string.

The second related problem is that you are writing a pointer address, v.file, and not the full string contents through the pipe. That is not correct because each process has a seperate address space and hence a pointer in one process is not valid in another process.

There are several ways to fix your problem. I will give you the simplest (but not the best).

First declare file inside the struct as a char array rather than a char pointer. This essentially gives you a fixed sized buffer.

#define MAX_FILENAME_LEN 64
typedef struct total {
  int words, lines, bytes;
  char file[MAX_FILENAME_LEN];
} Vals;

Then remove the malloc call. You don't need it anymore as file is already a buffer that you can copy into.

Finally, make sure you don't overflow the buffer during string copy:

if (strlen(fname) >= MAX_FILENAME_LEN) {
    error_exit("File name too long");
}
strcpy(v.file, fname);

You also don't need the +1 in the write as the sizeof gives you the full buffer size.

I'll leave it as an exercise for you to use dynamic memory for the file name in the struct. It's not hard but will require you to change your read and write logic a little as you will need to read/write the file name seperately (because writing the whole struct in that case will just write the pointer not the contents).

Segmentation fault 11 printing string from struct

3 Answers