Data writes over after realloc

Question

I a function that allows you to add question to a game. I use realloc to increase the memory so i can store more questions.

Script:

struct Question* AddQuestions(int* amountQuest){
    struct Question* questionsLocation = NULL;
    int startQuestion = 0;//How many question from before.
    int amount;
    if (*amountQuest > 0){
        startQuestion = *amountQuest;
    }

    system("cls");
    printf("How many statement do you want to add? ");
    scanf_s("%d", &amount);
    printf("\n");

    *amountQuest = amount + startQuestion;//Total amount of questions
    //Realloc or allocate new block if null
    questionsLocation = (struct Question*)realloc(questionsLocation,*amountQuest * sizeof(struct Question));

    if (questionsLocation != NULL){ // Check if allocating worked
        //Add questions
        for (int i = startQuestion; i < *amountQuest; i++){//Starts att startQuestion so I don't override the old question.
            printf("Statement %d: ", i + 1);
            fflush(stdin);
            fgets(questionsLocation[i].question, 200, stdin);

            printf("Dificulty: ");
            scanf_s("%d", &questionsLocation[i].difficult);

            printf("Right answer [1 = true / 0 = false] : ");
            scanf_s("%d", &questionsLocation[i].rightAnswer);

            printf("\n");
        }

        return questionsLocation;
    }
    else{
        printf("Problem occurred, try again");
        fflush(stdin);
        getchar();
        return NULL;
    }
}

When I add the new questions the old get override. Image: When I add the first questions without problem First image where everything works

Image: When I add more questions and I get problem. Second image where it fails

Something tells me knowing everything about `struct Question` would assist people in viewing your code. Post it please, ideally in an [MCVE](https://stackoverflow.com/help/mcve). I also see nothing that *previously* allocates to `questionsLocation`. In fact, `struct Question* questionsLocation = NULL;` is the only prior encounter, which is valid to send to `realloc`, but has no prior content, so I don't see how anything is being lost or overwritten from nothing. And stop `fflush`-ing `stdin`. It isn't standard. — WhozCraig, Dec 31 '14 at 11:23
Show us the code that calls `AddQuestions`. Also, please show the `Question` struct. — NPE, Dec 31 '14 at 11:24
`startQuestion` is not a static variable. It won't contain the number from before, it will be set to `0` every time you call the function. — Barmar, Dec 31 '14 at 11:25
Consider copying your input and output as *text* into your question. The images do not add anything useful, and are hard to read anyways. — Jongware, Dec 31 '14 at 11:43

NPE · Accepted Answer · 2014-12-31T11:34:34.563

Your code doesn't attempt to preserve the earlier questions. It always calls realloc() with a NULL pointer, resulting in a brand new allocation:

struct Question* questionsLocation = NULL;
/* ...there are no assignments to questionsLocation here... */
questionsLocation = (struct Question*)realloc(questionsLocation,
                                      *amountQuest * sizeof(struct Question));

To preserve the data, you need to hold on to the pointer you got from realloc() and use it next time you are reallocating memory.

One way to do this is to change the function to take questionsLocation as an argument:

struct Question* AddQuestions(int* amountQuest, struct Question* questionsLocation) {
    ...
    questionsLocation = (struct Question*)realloc(questionsLocation, ...);
    ...
    return questionsLocation;
}

and then use it like so:

int amountQuest = 0;
struct Question* questionsLocation = NULL;
...
questionsLocation = AddQuestion(&amountQuest, questionsLocation);

(For clarity, I did not include any error checking. You might want to add some.)

Barmar · Answer 2 · 2014-12-31T11:37:43.563

1

You need to declare questionsLocation and startQuestion to be static so they'll keep their values from one call to the next.

static struct Question* questionsLocation = NULL;
static int startQuestion = 0;//How many question from before.

However, a better approach in general is for your function to receive these values as parameters.

struct Question* AddQuestions(int* amountQuest, structQuestion*questionsLocation, int startQuestion){

edited Dec 31 '14 at 11:37

answered Dec 31 '14 at 11:29

Barmar

741,623
53
500
612

Without an explanation of pitfalls associated with `static` variables, are you sure this is good advice? – NPE Dec 31 '14 at 11:31

score 0 · Answer 3 · answered Dec 31 '14 at 11:28

Right way to realloc() is

The size should be able to accomodate both your previous questions as well as the new questions. For Eg:

n = num_of_old_questions + num_of_new_questions;

struct Question *temp = realloc(questionsLocation, sizeof(struct Question) * n); 

if(temp != NULL)
{
  questionsLocation = temp;
}

If realloc() fails then you will be loosing your original pointer data also so it is better to use temp for reallocation and check the return value before assigning it to the original pointer

Data writes over after realloc

3 Answers3