C - Array of Char Arrays

Question

Im trying to work with the example in the K and R book for this topic, but struggling.

I want an array of Char Arrays, whereby each element of the 'Father' Array points to an array of characters (string). Basically, I am reading from a file, line at a time, storing each line into an array, and then trying to store that array, into another array, which I can then sort via qsort.

But I can't seem to get anywhere with this! Anyhelp on my code is much appreciated, i.e. where to go from where I am!

EDIT: The problem is, the printing function isn't printing out my words that should be within the array of arrays, instead its just printing garbage, the main problem is, I'm not sure whether i am de-referencing things correctly, or not at all, whether I am adding it to the array of arrays correctly etc.

Regards.

#define MAXLINES 5000 /* max no. lines to be stored */
#define MAXLEN 1000 /* max length of single line */

char *lineptr[MAXLINES];

void writelines(char *lineptr[], int nlines);

int main(int argc, char *argv[]) {
 int nlines = 0, i, j, k;
 char line[MAXLEN];
 FILE *fpIn;
 fpIn = fopen(argv[1], "rb");
 while((fgets(line, 65, fpIn)) != NULL) {
     j = strlen(line);
     if (j > 0 && (line[j-1] == '\n')) {
         line[j-1] = '\0';
     }
     if (j > 8) {
         lineptr[nlines++] = line;
     }
 }  
 for(i = 0; i < nlines; i++)
     printf("%s\n", lineptr[i] );
return 0;    
}

Answer 1

Dan definitely found one error, the identical storage. But I think there are more bugs here:

while((fgets(line, 65, fpIn)) != NULL) {

Why only 65? You've got MAXLEN space to work with, you might as well let your input be a bit longer.

    j = strlen(line);
    if (j > 0 && (line[j-1] == '\n')) {
        line[j-1] = '\0';
    }
    if (j > 8) {
        lineptr[nlines++] = line;
    }
}

Why exactly j > 8? Are you supposed to be throwing away short lines? Don't forget to deallocate the memory for the line in this case, once you've moved to the dynamic allocation that Dan suggests.

Update

ott recommends strdup(3) -- this would be easy to fit into your existing system:

while((fgets(line, 65, fpIn)) != NULL) {
    j = strlen(line);
    if (j > 0 && (line[j-1] == '\n')) {
        line[j-1] = '\0';
    }
    if (j > 8) {
        lineptr[nlines++] = strdup(line);
    }
}

Dan recommended calloc(3), that would be only slightly more work:

line = calloc(MAXLINE, sizeof char);
while((fgets(line, 65, fpIn)) != NULL) {
    j = strlen(line);
    if (j > 0 && (line[j-1] == '\n')) {
        line[j-1] = '\0';
    }
    if (j > 8) {
        lineptr[nlines++] = line;
    line = calloc(MAXLINE, sizeof char);
    }
}

Of course, both these approaches will blow up if the memory allocation fails -- checking error returns from memory allocation is always a good idea. And there's something distinctly unbeautiful about the second mechanism.

Answer 2

A problem is that line[MAXLEN] is an automatic variable, and so each time through the while loop it refers to the same array. You should dynamically allocate line each time through the while loop (line = calloc(MAXLEN, sizeof(char)) before calling fgets). Otherwise fgets always writes to the same memory location and lineptr always points to the same array.