1

I have a project that involves reading in an indeterminate number of strings and appending them into different char** based on some associated metadata. I have code that will realloc() a char** to grow dynamically with the data, and it takes as input a pointer to one of the char** so it can be somewhat generic. However, I'm screwing something up with the pointers that causes realloc() to free() the char** too early, resulting in errors. I cannot find what I am doing wrong.

Here's a stripped-down sample that illustrates what I'm trying to do. References to metadata are stripped out, and instead the code alternates between one char** and another in a manner that might happen in the full project. This sample also omits some error checking on malloc() and some appropriate cleanup (ie. free()) that would be present in the full project.

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

void print_elems(char **array, int length) {
    for (int i = 0; i < length; i++) {
        printf("%s", array[i]);
    }
}

void main() {

    char **array = (char**) malloc(sizeof(char*));
    int length = 1;
    int index = 0;
    char **array2 = (char**) malloc(sizeof(char*));
    int length2 = 1;
    int index2 = 0;


    char **pointarray = array2;
    int* pointlen = &length2;
    int* pointidx = &index2;

    char newelem[10];
    while(1) {
        printf("Enter a string: ");
        fgets(newelem, 10, stdin);

        pointarray = (pointarray == array2 ? array : array2);
        pointlen = (pointlen == &length2 ? &length : &length2);
        pointidx = (pointidx == &index2 ? &index : &index2);

        if (*pointlen == *pointidx) {
            printf("Resizing array...\n");
            void* newarray = realloc(pointarray, sizeof(char*)*(*pointlen+1));
            if (pointarray == NULL) {
                perror("Error allocating memory.\n");
                exit(1);
            } else {
                pointarray = (char**) newarray;
            }
            (*pointlen)++;
        }

        pointarray[*pointidx] = strdup(newelem);
        (*pointidx)++;

        print_elems(pointarray, *pointlen);
    }

}

Typically after no more than 10 runs through the loop the program crashes. Valgrind gives this output:

==11278== Invalid free() / delete / delete[] / realloc()
==11278==    at 0x483AD19: realloc (vg_replace_malloc.c:836)
==11278==    by 0x4012EA: main (test.c:38)
==11278==  Address 0x4a23090 is 0 bytes inside a block of size 8 free'd
==11278==    at 0x483AD19: realloc (vg_replace_malloc.c:836)
==11278==    by 0x4012EA: main (test.c:38)
==11278==  Block was alloc'd at
==11278==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==11278==    by 0x401215: main (test.c:17)
==11278== 
==11278== Invalid write of size 8
==11278==    at 0x401345: main (test.c:48)
==11278==  Address 0x10 is not stack'd, malloc'd or (recently) free'd

If I don't do all this pointer-switching the program runs fine, but the project would be a lot more complicated and I have to imagine there's a way to do what I'm trying to do.

Can someone tell me what I'm screwing up that makes realloc() go off the rails?

Paul
  • 33
  • 4
  • Welcome to Stack Overflow! [dont cast malloc](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – Barmar May 05 '20 at 20:07
  • Why do you have two lists? What exactly are the assignments to `pointarray`, `pointlen`, and `pointidx` supposed to be doing? – dbush May 05 '20 at 20:13
  • @dbush He's apparently trying to put alternate inputs into each array, using indirection to switch between them. – Barmar May 05 '20 at 20:14
  • @Barmar I get that, the question is what is the criteria for choosing each one supposed to be? – dbush May 05 '20 at 20:15
  • I don't see any criteria, it just goes back and forth. – Barmar May 05 '20 at 20:19
  • @dbush - it's just sample code that switches back and forth between the lists to illustrate the error I'm getting. The full project looks at other data to determine how to sort the input. – Paul May 05 '20 at 20:28

1 Answers1

1

After you call realloc() you assign the result to pointarray, but this doesn't change array or array2. Then during a future iteration, you assign one of them to pointarray, but they no longer point to valid storage.

You need an extra level of indirection, similar to the way you indirect to get to the length and index variables.

Also, after you call realloc() you're checking pointarray, but you should be checking newarray.

void main() {

    char **array = malloc(sizeof(char*));
    int length = 1;
    int index = 0;
    char **array2 = malloc(sizeof(char*));
    int length2 = 1;
    int index2 = 0;

    char ***pointarray = array2;
    int* pointlen = &length2;
    int* pointidx = &index2;

    char newelem[10];
    while(1) {
        printf("Enter a string: ");
        fgets(newelem, 10, stdin);

        pointarray = (pointarray == &array2 ? &array : &array2);
        pointlen = (pointlen == &length2 ? &length : &length2);
        pointidx = (pointidx == &index2 ? &index : &index2);

        if (*pointlen == *pointidx) {
            printf("Resizing array...\n");
            void* newarray = realloc(*pointarray, sizeof(char*)*(*pointlen+1));
            if (newarray == NULL) {
                perror("Error allocating memory.\n");
                exit(1);
            } else {
                *pointarray = newarray;
            }
            (*pointlen)++;
        }
        (*pointarray)[*pointidx] = strdup(newelem);
        (*pointidx)++;

        print_elems(*pointarray, *pointlen);
    }
}
Barmar
  • 741,623
  • 53
  • 500
  • 612
  • Thanks - this gives a bit of help but doesn't work. It crashes on the third interation with "realloc(): invalid pointer" – Paul May 05 '20 at 20:26
  • Scratch that - I had an error in my code. It does work. Thanks for the help, and the pointers on malloc() ! – Paul May 05 '20 at 20:30