0

I am really having a problem understanding dynamically allocated arrays.

I am attempting to read a text file of strings to a 2d array so I can sort them out later. right now as my code stands it throws seg faults every once in a while. Which means I'm doing something wrong. I've been surfing around trying to get a better understanding of what malloc actually does but I want to test and check if my array is being filled.

my program is pulling from a text file with nothing but strings and I am attempting to put that data into a 2d array.

for(index = 0; index < lines_allocated; index++){
//for loop to fill array 128 lines at a time(arbitrary number)
    words[index] = malloc(sizeof(char));
    if(words[index] == NULL){
        perror("too many characters");
        exit(2);
    }
    //check for end of file
    while(!feof(txt_file)) {
        words = fgets(words, 64, txt_file);
        puts(words);
        //realloc if nessesary
        if (lines_allocated == (index - 1)){
            realloc(words, lines_allocated + lines_allocated);
        }
    }

}
//get 3rd value placed
printf("%s", words[3]);

since this just a gist, below here ive closed and free'd the memory, The output is being displayed using puts, but not from the printf from the bottom. an ELI5 version of reading files to an array would be amazing.

Thank you in advance

gemini88mill
  • 170
  • 2
  • 11
  • 1
    [while-!feof()-is-always-wrong](http://stackoverflow.com/a/5432517/3386109) – user3386109 Sep 15 '15 at 22:24
  • 1
    Two observations. (1) `words[index] = malloc(sizeof(char));` is allocating 1 byte. (2) `while(!feof(txt_file))` does not do what you think it does. – Weather Vane Sep 15 '15 at 22:25
  • 2
    Also `words = fgets(words, 64, txt_file);` is wrong. The `feof` loop should be `while(fgets(words, 64, txt_file) != NULL) { ... }` – Weather Vane Sep 15 '15 at 22:28
  • hmm... ok that makes a lot of sense. so if I do 'words[index] = malloc(sizeof(char) * sizeof chunk);' then it will allocate 64 bytes because chunk = 64. – gemini88mill Sep 15 '15 at 22:34
  • 1
    I think you have a confusion with your use of `words`. On the one hand it's an array of `char*` pointers to which you allocate memory (but should be more than one byte), but when you read in the string, you read it into `words` instead of `words[index]`. Are you ignoring compiler warnings? – Weather Vane Sep 15 '15 at 22:37
  • @Weather Vane fixed the feof, works the same, I guess my big question is how to access the array later... – gemini88mill Sep 15 '15 at 22:39
  • Clion isnt giving me any warnings at the moment... – gemini88mill Sep 15 '15 at 22:41

2 Answers2

1

void *malloc(size_t n) will allocate a region of n bytes and return a pointer to the first byte of that region, or NULL if it could not allocate enough space. So when you do malloc(sizeof(char)), you're only allocating enough space for one byte (sizeof(char) is always 1 by definition).

Here's an annotated example that shows the correct use of malloc, realloc, and free. It reads in between 0 and 8 lines from a file, each of which contains a string of unknown length. It then prints each line and frees all the memory.

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

/* An issue with reading strings from a file is that we don't know how long
   they're going to be.  fgets lets us set a maximum length and discard the
   rest if we choose, but since malloc is what you're interested in, I'm
   going to do the more complicated version in which we grow the string as
   needed to store the whole thing.  */
char *read_line(void) {
    size_t maxlen = 16, i = 0;
    int c;

    /* sizeof(char) is defined to be 1, so we don't need to include it.
       the + 1 is for the null terminator */
    char *s = malloc(maxlen + 1);
    if (!s) {
        fprintf(stderr, "ERROR: Failed to allocate %zu bytes\n", maxlen + 1);
        exit(EXIT_FAILURE);
    }

    /* feof only returns 1 after a read has *failed*.  It's generally
       easier to just use the return value of the read function directly.
       Here we'll keep reading until we hit end of file or a newline.  */
    while ('\n' != (c = getchar())) {
        if (EOF == c) {
            /* We return NULL to indicate that we hit the end of file
               before reading any characters, but if we've read anything,
               we still want to return the string */
            if (0 == i) return NULL;
            break;
        }

        if (i == maxlen) {
            /* Allocations are expensive, so we don't want to do one each
               iteration.  As such, we're always going to allocate more than
               we need.  Exactly how much extra we allocate depends on the
               program's needs.  Here, we just add a constant amount. */
            maxlen += 16;

            /* realloc will attempt to resize the memory pointed to by s,
               or copy it to a newly allocated region of size maxlen.  If it
               makes a copy, it will free the old version. */
            char *p = realloc(s, maxlen + 1);
            if (!p) {
                /* If the realloc fails, it does not free the old version, so we do it here. */
                free(s);
                fprintf(stderr, "ERROR: Failed to allocate %zu bytes\n", maxlen + 1);
                exit(EXIT_FAILURE);
            }
            s = p;//set the pointer to the newly allocated memory
        }

        s[i++] = c;
    }
    s[i] = '\0';

    return s;
}

int main(void) {
    /* If we wanted to, we could grow the array of strings just like we do the strings
       themselves, but for brevity's sake, we're just going to stop reading once we've
       read 8 of them. */
    size_t i, nstrings = 0, max_strings = 8;

    /* Each string is an array of characters, so we allocate an array of char*;
       each char* will point to the first element of a null-terminated character array */
    char **strings = malloc(sizeof(char*) * max_strings);
    if (!strings) {
        fprintf(stderr, "ERROR: Failed to allocate %zu bytes\n", sizeof(char*) * max_strings);
        return 1;
    }

    for (nstrings = 0; nstrings < max_strings; nstrings++) {
        strings[nstrings] = read_line();
        if (!strings[nstrings]) {//no more strings in file
            break;
        }
    }

    for (i = 0; i < nstrings; i++) {
        printf("%s\n", strings[i]);
    }

    /* Free each individual string, then the array of strings */
    for (i = 0; i < nstrings; i++) {
        free(strings[i]);
    }
    free(strings);

    return 0;
}
Ray
  • 1,706
  • 22
  • 30
0

I haven't looked too closely so I could be offering an incomplete solution.

That being said, the error is probably here:

realloc(words, lines_allocated + lines_allocated);

realloc if succesful returns the new pointer, if you're lucky it can allocate the adjacent space (which wouldn't cause a segfault).

words = realloc(words, lines_allocated + lines_allocated);

would solve it, although you probably need to check for errors.

Work of Artiz
  • 1,085
  • 7
  • 16