program crashing when trying to add to a dynamic array

Question

I am trying to write a function that will add a set of data (first name, last name, score) into 3 different dynamic arrays (one 2d char array for the first name, one 2d char array for the the last name, and a float array for the score). Here is my code so far:

void add(char **firstname, char **lastname,char *newfirst,char *newlast,float newscore, float *score, int *num)
{
    realloc(firstname, sizeof(newfirst));
    realloc(lastname, sizeof(newlast));
    realloc(score, sizeof(float)*sizeof(newscore));
    *num = *num + 1;
    firstname[*num] = newfirst;
    lastname[*num] = newlast;
    score[*num] = newscore;


}

I know that I need to reallocate memory in order to add anything to the arrays. I am trying to add 1 to num so that whenever I run the other functions (such as printing and sorting, etc.) it will run through the other loops that are in those functions the appropriate amount of times. All other aspects of the program work, it just crashes when I run it through this function. Am I allocating the memory properly, and what could be causing the crash?

Here is the case in main, in case anyone needs to look at it:

case 2: printf("Enter record of student to add: \n");
            printf("Enter the first name: \n");
            scanf_s("%s", addfirst, 21);
            printf("Enter the last name: \n");
            scanf_s("%s", addlast, 21);
            printf("Enter the score:\n");
            scanf_s("%f", addscore);
            add(firstname, lastname, addfirst, addlast, addscore, score, num);
            break;

I am 99% sure I have properly initialized all of the variables used for this function.

Answer 1

You're using realloc incorrectly. You must accept the return value of realloc in a variable different from that used as the first parameter since realloc can potentially return NULL. If it isn't NULL, you can assign it to the original variable; but if it's NULL, you still have a pointer to the previous memory (avoiding a memory leak).

There are other things wrong with your code.

sizeof does not return the length of a string, but the size of the object the variable references, in this case it's a pointer, so it'll return 4 or 8. Use strlen instead.

You're using firstname and lastname as if they're local variables. The values you assign to them disappear when the function returns. You'll need to pass in a triple pointer and dereference them on use.

You are trying to realloc space for a string, but you haven't attempted to reallocate any space for another pointer in the array to actually point to the new string memory.

You can't copy a string with a simple assignment statement; use strcpy instead.

And you're not passing the variables correctly, either. If they are to (potentially) receive new values, you need to pass the address, not the value.

So, it looks like you're trying to do this:

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

void add(
    char *** firstname,  char *** lastname, float ** score,
    char   * newfirst,   char   * newlast,  float    newscore,
    int      index)
{
    char ** t;
    float * f;

    t = realloc(*firstname, (index + 1) * sizeof(*firstname));
    if (!t) ; // handle error
    *firstname = t;
    (*firstname)[index] = malloc(strlen(newfirst) + 1);
    strcpy((*firstname)[index], newfirst);

    t = realloc(*lastname, (index + 1) * sizeof(*lastname));
    if (!t) ; // handle error
    *lastname = t;
    (*lastname)[index] = malloc(strlen(newlast) + 1);
    strcpy((*lastname)[index], newlast);

    f = realloc(*score, (index + 1) * sizeof(*score));
    if (!f) ; // handle error
    *score = f;
    (*score)[index] = newscore;
}

int main() {
    char **firstname = NULL, **lastname = NULL;
    float *score = NULL;
    int num = 0;

    add(&firstname, &lastname, &score, "one",   "two",  1.1f, num++);
    add(&firstname, &lastname, &score, "three", "four", 2.2f, num++);
    add(&firstname, &lastname, &score, "five",  "six",  3.3f, num++);

    for (int i = 0; i < num; ++i)
        printf("%s %s %f\n", firstname[i], lastname[i], score[i]);

    return 0;
}

Note that only the pointer arrays are realloced. The string memory is simply malloced.

It's pretty inefficient to realloc for a single element each time. Normally you'd realloc a chunk of elements at a time. Such an array is best represented as a struct so that its currently allocated space and currently occupied space can be saved together with the data.

Answer 2

This use of realloc is incorrect:

realloc(firstname, sizeof(newfirst));

The problem is that realloc returns an address, which may be different from the original, which must be assigned back to the pointer being reallocated, or to another pointer (the second case is rare).

The problem is that when realloc allocates a different block, it frees firstname, too. The end result is that your firstname pointer points to a freed block, while the reallocated block becomes a memory leak.

Moreover, you are calculating the size to reallocate incorrectly: sizeof gives you the size of the pointer, while you are looking for the length of the string.

Finally, firstname is a pointer to pointer to character (which is understandable, because you want to modify the pointer in the caler). You need to dereference it on assignment.

Here is how you can fix these problems:

*firstname = realloc(*firstname, strlen(newfirst)+1);