Why is the pointer "Start" changing? (C)

Question

I have a function to read a load of words from a file and store them in a linked list. The struct for which is:

typedef struct node {
  char * word;
  int wordLength;
  int level;
  struct node * parent;
  struct node * next;
} Node;

In this function, I have Start->word and Current->word pointing to the first word in the list. Then, cycle through the rest of the words in the file, storing them into Current. I want to keep Start, pointing to the start of the list, but, when I print out the value of Start->word at the end of the function, it's value has changed to the last word in the file stream. This code works fine if I statically assign the max length of Node->word and currentWord, however, the code is meant to make no assumptions about the maximum word length. Here is the funciton:

Node * GetWords(char * dictionary, Node * Current, int * listLength, int maxWordLength)
{
  Node * Start = (Node *)malloc(sizeof(Node));
  FILE *fp;
  char * currentWord = (char *)malloc(maxWordLength * sizeof(char));
  fp = fopen(dictionary, "r");

  ErrorCheckFile(fp);

  if((fscanf(fp, "%s", currentWord)) != EOF){
    Current = Start = AllocateWords(currentWord);
  }
  //If I print out the value of "Start" here, it is correct.
  while((fscanf(fp, "%s", currentWord)) != EOF){
    Current->next = AllocateWords(currentWord);
    Current = Current->next;
    (*listLength)++;
  }
  printf("Starting: %s\n", Start->word); //If I print out the value of
  //"Start" here, it is set to the same value as "Current", which is the
  //last word in the file. I need to keep "Start" constantly pointing to
  // the start to I can reset "Current" to the start throughout the program.

  fclose(fp);
  return Start;
}

Here is my AllocateWords():

Node * AllocateWords(char * string)
{
  Node * p;
  p = (Node *)malloc(sizeof(Node));
  if(p == NULL){
    fprintf(stderr, "ERROR: Cannot allocate space...\n\n");
    exit(1);
  }
  p->word = string;
  p->level = -1;
  p->parent = NULL;
  p->wordLength = strlen(p->word);
  p->next = NULL;
  return p;
}

It seems pretty obvious to me that `Start` is changing because you write `Current = Start = AllocateWords(currentWord);`, so yeah, `Start` is modified. — MicroVirus, Nov 22 '15 at 16:13
it is hard to answer without AllocateWords function. This line will cause memory leak: Current = Start = AllocateWords(currentWord); Because your original Start will be lost. It feels that your currentWord is the same object for all nodes, and will be overridden so your list will hold all the same word. — VladimirS, Nov 22 '15 at 16:14

score 2 · Accepted Answer · edited May 23 '17 at 11:44

All nodes point to the same string, so you want to change the function AllocateWords to:

Node * AllocateWords(char * string)
{
  Node * p;
  p = (Node *)malloc(sizeof(Node));
  if(p == NULL){
    fprintf(stderr, "ERROR: Cannot allocate space...\n\n");
    exit(1);
  }
  p->word = strdup(string); //get a copy of the string
  p->level = -1;
  p->parent = NULL;
  p->wordLength = strlen(p->word);
  p->next = NULL;
  return p;
}

strdup will fix it, but you may want to consider writing your own copy of strdup for reasons mentioned here

Also strdup can fail, so you'll have to add some error checking.

Additionally, your program is leaking memory, as suggested by other users.

score 0 · Answer 2 · answered Nov 22 '15 at 16:13

0

You are overwriting the value of Start in this line:

Current = Start = AllocateWords(currentWord);

answered Nov 22 '15 at 16:13

simpel01

1,792
12
13

Should I write those 2 assignments separately? – liamw9 Nov 22 '15 at 16:15
I am not sure what was your intention here but if you want to keep `Start` pointing at the old position then you just need to do: `Current = AllocateWords(currentWord);` – simpel01 Nov 22 '15 at 16:17
I would like start to keep pointing at the first position in the list. If I do what you are suggesting, then Start isn't pointing to anything – liamw9 Nov 22 '15 at 16:20
`Node * Start = (Node *)malloc(sizeof(Node));` Start is already pointing to some memory you have allocated – simpel01 Nov 22 '15 at 16:23
Okay. In that case, how would I go about getting Start to point to the start of the list (which is initially where Current is pointing)? – liamw9 Nov 22 '15 at 16:28

Why is the pointer "Start" changing? (C)

2 Answers2