0 
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
void sortString(const char* input, char* output);
int cmpstr(void const *a,void const *b);
int readAllWords(FILE* f, char*** res, int * num_read);
int main (int argc, char ** argv)
{
   char **wordList;
   FILE* fid;
   int numWords;
   fid = fopen(argv[1],"r");
  readAllWords(fid, &wordList,&numWords);
}

int readAllWords(FILE* f, char*** res, int * num_read)
{
    char buffer[128];
    *num_read = 0;
    int size;
    while(fgets(buffer,128,f))
    {
       *num_read = *num_read +1;
       size = strlen(buffer);
       res = (char***)malloc(sizeof(char**));
       *res = (char **)realloc(*res,sizeof(char*)*(*num_read));
       (*res)[(*num_read)-1] = (char *)realloc((*res)[(*num_read)-1],sizeof(char)*size);
        strcpy((*res)[(*num_read)-1],buffer);
      printf("%s\n",(*res)[(*num_read)-1]);
    }
    printf("%s\n",(*res)[0]);

}

The values are storing and it prints out inside the while loop. But after the while loop, it cannot print out the strings.

The File is given in the main function. Do not understand why realloc is causing the loss of data?

Improve this question
5
  • 3
    res = (char***)malloc(sizeof(char**)); should be an alert as to the fact that you are doing it wrong. Commented Jan 19, 2014 at 23:57
  • Do NOT cast the return value of malloc()! Commented Jan 19, 2014 at 23:58
  • It would help to know how you're calling this function. Commented Jan 19, 2014 at 23:59
  • Looks like you aren't allocating an extra spot for the \0 in the realloc, but I agree with @H2CO3, (char***) is a sign that you need to refactor... Commented Jan 20, 2014 at 0:01
  • The thing is that I cannot change the function parameters so what is the proper way to initialize an array of strings? Commented Jan 20, 2014 at 0:04

2 Answers 2

Reset to default
1

One problem is that the code doesn't initialize res in main(), so you attempt to realloc() an indeterminate value. Either NULL or a value previously returned by malloc() or realloc() (or calloc()) would be OK, but since you pass an indeterminate value, you are invoking undefined behaviour, and a crash is a valid response to doing that.

However, there's a lot of other code in the function that should be reviewed as well.

This code works, and gets a clean bill of health from valgrind.

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

void readAllLines(FILE *f, char ***res, int *num_read);

int main(int argc, char **argv)
{
    char **wordList = 0;
    FILE *fid;
    int numLines = 0;
    if (argc > 1 && (fid = fopen(argv[1], "r")) != 0)
    {
        readAllLines(fid, &wordList, &numLines);
        fclose(fid);
        for (int i = 0; i < numLines; i++)
            printf("%d: %s", i, wordList[i]);
        for (int i = 0; i < numLines; i++)
            free(wordList[i]);
        free(wordList);
    }
    return 0;
}

void readAllLines(FILE *f, char ***res, int *num_read)
{
    char buffer[128];
    int size;
    while (fgets(buffer, sizeof(buffer), f))
    {
        *num_read = *num_read + 1;
        size = strlen(buffer) + 1;
        char **space = (char **)realloc(*res, sizeof(char *) * (*num_read));
        if (space == 0)
            return;
        *res = space;
        (*res)[*num_read - 1] = (char *)malloc(sizeof(char) * size);
        if ((*res)[*num_read - 1] == 0)
            return;
        strcpy((*res)[*num_read - 1], buffer);
        printf("%s\n", (*res)[*num_read - 1]);
    }
    printf("%s\n", (*res)[0]);
}
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

1

Possible reason for a segmentation fault:

 res = (char***)malloc(sizeof(char**));
 *res = (char **)realloc(*res,sizeof(char*)*(*num_read));

In the second line you try to reallocate whatever *res is pointing to. However since you did not initialize *res this could be anything. This will work only if *res == NULL. I guess it should be malloc, not realloc.

Other problems:

You allocate everything new in each loop iteration. This is a huge memory leak.

You already pass a valid memory address pointing to an char** by res, you shouldn't allocate for it again. It is an out parameter. (Remove the malloc call)

You need an initial malloc for *res before the loop (Or set *res = NULL).

The second realloc for *res[...] should be a malloc, because you never actually reallocate here. Also instead of allocating size bytes, you should allocate size+1 bytes for the terminating \0.

Your function has no return statement although it is non-void.

Improve this answer

3 Comments

But I need to increase the size of the array of pointers bc it is an array of strings.
do i need not set up **res for the character array?
@user3213348: That pair of lines is another problem. Yet another problem is that you only allocate enough space for size characters, not size+1. Always remember to account for the null byte at the end of a string; strlen() does not count that byte.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.