Segmentation Fault with Malloc

Question

I needed to implement and specific ADT, strqueue, for my CS class today and so I wrote up two functions: create_StrQueue(), and add_to_back(StrQueue sq, const char* str). Unfortunately, when I call create_StrQueue in add_to_back I get a seg fault, and I am unable to figure out why exactly. Here is the code that I wrote for these two functions:

[edit] I should probably malloc tempWord in add_to_back.

#include <stdlib.h>

// A strqueue is an ADT consisting of words
struct strqueue{
  StrQueue back;    // last StQueue in queue
  StrQueue next;    // next StrQueue in queue

  char* word;       // stored string
  int length;       // length of entire queue
};

typedef struct strqueue* StrQueue;

StrQueue create_StrQueue(void){

  StrQueue retq = malloc(sizeof(struct strqueue));  // get memory for a new strqueue
  retq->word = malloc(sizeof(char*)); 
  retq->word = NULL;
  retq->back = retq;       // set back pointer to itself
  retq->next = NULL;       // nothing after this strqueue yet

  return retq;
}

void add_to_back(StrQueue sq, const char* str){

  char* tempWord;
  sq->length++;

  for(int i=0; str[i]; ++i) tempWord[i]=str[i];  // copy string for the new strqueue

  if(sq->word==NULL) sq->word = tempWord;  // input strqueue was empty

  // input StrQueue was not empty, so add a new StrQueue to the back
  StrQueue new = create_StrQueue(); // results in seg fault
  new->word = tempWord;
  sq-back->next = new;  // swaping pointers around to add malloced StrQueue to the back
  sq->back = next;
}

I'm at a loss, and so I'm hoping someone can clarify what exactly is going on, because when I run main like so;

int main(void){

char* str1 = "Hello";

StrQueue sq = create_StrQueue(); // does not cause seg fault
add_to_back(sq, str1);
}

calling create_StrQueue() for the first time works just fine.

Answer 1

char* in the struct is a pointer to a character array. retq->word = malloc(sizeof(char*)); is not the right way to allocate a string; what this actually does is it assigns a tiny array to word, essentially useless, and then you overwrite what you just allocated by assigning NULL to word, leaking the memory. All memory allocated by malloc must be manually released later on with free. You are dealing with a pointer. Assigning data to it has no magic involved in C, you are simply replacing the value of the pointer itself.

In add_to_back, you need to allocate space to tempWord before copying data into it:

tempWord = malloc( strlen(str)+1 );

You add 1 to accommodate the null terminator in the string. Use strcpy to copy into tempWord instead of writing your own string copying method there, your method does not add a null terminator.

An even better solution would be to have create_StrQueue accept a const char* parameter, and do the string allocation and copying in there.

You should also avoid using the word new since that looks a bit confusing to a c++ programmer. :)