CODEWITHSUNDEEP
cpointersrealloc
nugh
nugh

Reputation: 135

realloc, for string array in C

Is there a correct way to use realloc, for when you want to add words of variable size to a string array? I am getting a segmentation fault. Please show me what's wrong

// This function puts every word found in a text file, to a String array, **words
char **concordance(char *textfilename, int *nwords){
    FILE * fp;
    char *fileName = strdup(textfilename);
    fp = fopen(fileName, "r");
    if(fp == NULL) {
        perror("fopen");
        exit(1);
    }
    char **words = malloc(sizeof(char));
    // char **words = NULL

    char line[BUFSIZ];
    while(fgets(line, sizeof(line), fp) != NULL){
        char *word = strdup(line);
        word = strtok(word, " ");
        do{
            words = realloc(words, (*nwords+1) * sizeof(char(*)));
            words[*nwords] = word;
        } while((word = strtok(NULL, " ")) != NULL);
    }
    return words;
}


int main(int argc, const char * argv[]) {
    int *nwords = malloc(sizeof(int));
    nwords = 0;
    concordance("test.txt", nwords);
}

Upvotes: 1

Views: 2861

Answers (2)

ram914
ram914

Reputation: 331

You seem to initialize nwords to 0, in a wrong way. As you have declared it as a pointer, you can not access it directly. instead, you should use the de-reference operator *.

make the following change in the main function

*nwords = 0; instead of nwords = 0;

nwords = 0 modifies the location to which nwords is pointing to, to the location with address 0, to which you have no access and can not assign.

WARNING:

  1. It is better not to perform realloc on the same pointer, it will make the pointing location NULL if the realloc fails, leading to the loss of the previously existing data. Instead, as @David suggests, you could use a temp variable to realloc memory and then, check if it is not NULL and then assign its contents to the words pointer.
    //your code
    char *tmp = realloc(words, /* new size*/);
    if(tmp != NULL)
        words = tmp;
    // your code
  1. while using realloc you usually use it to allocate a block of data, not for a single location.

Upvotes: 1

Antonio Ragagnin
Antonio Ragagnin

Reputation: 2327

When you initiate the value of nwords, you were overwriting its pointer address, not its value.

Additionally, as the commenter says, the line char **words = malloc(sizeof(char)); is not correct. But you always re-allocate the variable words so the code still works as expected. To make it super safe you should change it to char **words = malloc(sizeof(char*));

I use the line *nwords = 0; and now it works as expected.

#define BUFSIZ 1000
#include<stdio.h>
// This function puts every word found in a text file, to a String array, **words
char **concordance(char *textfilename, int *nwords){
  FILE * fp;
  char *fileName = strdup(textfilename);
  fp = fopen(fileName, "r");
  if(fp == NULL) {
    perror("fopen");
    exit(1);
  }
  char **words = malloc(sizeof(char));
  // char **words = NULL

  char line[BUFSIZ];
  while(fgets(line, sizeof(line), fp) != NULL){
    char *word = strdup(line);
    word = strtok(word, " ");
    printf("word='%s'\n",word);
    do{
      *nwords=*nwords+1;
      printf("nwords=%d\n",*nwords);
      words = realloc(words, (*nwords+1) * sizeof(char(*)));
      words[*nwords] = word;
    } while((word = strtok(NULL, " ")) != NULL);
  }
  return words;
}


int main(int argc, const char * argv[]) {
  int *nwords = malloc(sizeof(int));
  *nwords = 0;
  concordance("test.txt", nwords);
}

Upvotes: 0

Related Questions