0

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.

user3570936
  • 3
  • 3
  • How are `firstname` and `lastname` defined? – ooga Apr 26 '14 at 23:37
  • firstname and lastname are defined in main like this: `char **firstname=NULL;` `char **lastname=NULL;` `firstname = malloc(num * (sizeof(char*)));` `for (i = 0; i < num; i++) { firstname[i] = malloc(21); }` `lastname = malloc(num * (sizeof(char*)));` `for (i = 0; i < num; i++) { lastname[i] = malloc(21); } ` – user3570936 Apr 26 '14 at 23:38

2 Answers2

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.

ooga
  • 15,423
  • 2
  • 20
  • 21
  • yes, that's essentially what I want to do, thank you for writing out the code. I understand what is going on, but unfortunately, it still crashes after input. Also, there is probably no need for the null check, as there will always be data in the arrays; before any function is run, the program asks for at least one first name, last name, and score. – user3570936 Apr 27 '14 at 00:09
  • There is absolutely a necessity for the null check just as there's a necessity to check a file pointer for null after opening a file. You can leave it out if you want, but at least realize that it would be an error to leave it out in *real* code. It has nothing to do with your data. Anyway, I fixed a couple of bugs, so try it again. – ooga Apr 27 '14 at 00:11
  • Debugging OP's program for him is a major disservice to the person whom you are trying to help. By writing his code for him you taking away the learning opportunity from him. – Sergey Kalinichenko Apr 27 '14 at 00:19
  • after reading some more about null checking, I can see what you are saying; thank you for pointing that out. The program is still crashing after the input, and the way that it is crashing (this program has stopped working) leads me to believe that there is still something wrong with the memory allocation and reallocation, but I can't pinpoint where the problem would be in the function. – user3570936 Apr 27 '14 at 00:22
  • @user3570936 All I can say is that the code above does not crash for me and I cannot detect any errors in it. There may be errors, but I cannot see them. Good night and good luck! – ooga Apr 27 '14 at 00:24
  • @ooga If you think that the way I tech OP to use realloc incorrect, please indicate that in the comment to **my** answer. You did not address my point about writing the code for OP. Don't you think it's just wrong? – Sergey Kalinichenko Apr 27 '14 at 00:25
  • ah, the problem was in one of my printf statements, I forgot the & before addscore. Thanks again for your help and insight. – user3570936 Apr 27 '14 at 00:27
  • @dasblinkenlight I agree that I shouldn't have written the code for the OP. I was being lazy (and a little drunk). It won't happen again. – ooga Apr 27 '14 at 19:04
0

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);
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523