0

I have a function in C that adds a new Question to the head of a singly linked list:

int AddQuestion()
{
    unsigned int aCount = 0;
    Question* tempQuestion = malloc(sizeof(Question));

    tempQuestion->text = malloc(500);

    fflush(stdin);
    printf("Add a new question.\n");
    printf("Please enter the question text below:\n");
    fgets(tempQuestion->text, 500, stdin);
    printf("\n");
    fflush(stdin);
    printf("How many answers are there?: ");
    scanf("%u", &tempQuestion->numAnswers);
    fflush(stdin);
    tempQuestion->answers = malloc(sizeof(Answer*) * tempQuestion->numAnswers);
    for (aCount = 0; aCount < tempQuestion->numAnswers; aCount++)
    {
        tempQuestion->answers[aCount] = malloc(sizeof(Answer));
        tempQuestion->answers[aCount]->content = malloc(250);
        printf("Enter answer #%d: \n", (aCount + 1));
        fgets(tempQuestion->answers[aCount]->content, 250, stdin);
        fflush(stdin);
        printf("Is it correct or wrong? correct = 1, wrong = 0: ");
        scanf("%u", &tempQuestion->answers[aCount]->status);
        fflush(stdin);
    }

    tempQuestion->pNext = exam.phead;
    exam.phead = tempQuestion;

    printf("\n");
    fflush(stdin);

    return 1;
}

As you can see, I am using malloc() to allocate the space I need for the new question. However, if I try to call free() on tempQuestion, it removes the question from the exam. I do not want to remove the question unless the question is deleted or the program terminates.

I have a cleanup function that is supposed to free() up all the used memory, but it does not free up tempQuestion in the addQuestion() function.

void CleanUp()
{
    unsigned int i = 0;
    Question* tempQuestion = NULL;

    if (exam.phead != NULL) {
        while (exam.phead->pNext != NULL) {
            tempQuestion = exam.phead;
            exam.phead = tempQuestion->pNext;
            for (i = 0; i < tempQuestion->numAnswers; i++) {
                free(tempQuestion->answers[i]->content);
                free(tempQuestion->answers[i]);
            }
            free(tempQuestion->pNext);
            free(tempQuestion->text);
            free(tempQuestion);
        }
        free(exam.phead);
    }
}

How would I free() tempQuestion in my addQuestion() function so that it only frees the memory when execution ends? I'm using Visual Studio C++ 2012 but I have to write using only C syntax (no C++). I am fairly new to C programming as well. Thanks!

Benjamin
  • 305
  • 2
  • 3
  • 14

3 Answers3

1

To answer your question: You don't. You should not call free(tempQuestion) in the AddQuestion function, because that free the memory you just allocated.

When you assign a pointer to another pointer, it's only the pointer that is copied, not what it points to.

Some programmer dude
  • 380,411
  • 33
  • 383
  • 585
  • That is what I thought, but I was unsure. Is there a way then that I could release the memory when execution ends (i.e. inside CleanUp())? – Benjamin Sep 10 '14 at 13:12
  • @BenjaminC.Huskisson-Snider: Why bother freeing memory at all when the program exits? Does your platform not free all the programs resources then? – Deduplicator Sep 10 '14 at 13:21
  • @BenjaminC.Huskisson-Snider It looks to me like you're already doing it. But you also do call `free` on pointers you shouldn't, like `tempQuestion->pNext` (this causes [*undefined behavior*](http://en.wikipedia.org/wiki/Undefined_behavior) in the next iteration of the loop when you dereference the now unallocated pointer). On the other hand, you forget to free `tempQuestion->answers`. – Some programmer dude Sep 10 '14 at 13:28
  • @JoachimPileborg: The now *indeterminate* (or dangling, but certainly not unallocated) pointer. I really hope the auto-storage-class variable is still allocated, or the implementation has a huge bug. – Deduplicator Sep 10 '14 at 13:31
0

Allocate and deallocate memory for tempQuestion in main instead of allocating in AddQuestion functions.

int main ()
{
    Question *tempQuestion = malloc (sizeof (Question));
    .
    .
    .
    free (tempQuestion);
    return 0;
}

This will help you to deallocate the memory for tempQuestion just before you return from main.

Also while deallocating memory for tempQuestion->answer you missed out this

for (i = 0; i < tempQuestion->numAnswers; i++) {
    free(tempQuestion->answers[i]->content);
    free(tempQuestion->answers[i]);
}
free (tempQuestion->answer);

Rest should be ok.

Adarsh
  • 883
  • 7
  • 18
  • Please read: https://stackoverflow.com/questions/25766121/how-would-i-free-memory-allocated-to-a-pointer-in-c#comment40292911_25766121 – Deduplicator Sep 10 '14 at 13:19
  • Casting the return-value of `malloc` is at best bad style, and an unneccessary source of errors. And passing a type to `sizeof` instead of the dereferenced target-pointer is a bit more error-prone than neccessary. Read the [Q&A to casting the return-value of `malloc`](http://stackoverflow.com/q/605845) – Deduplicator Sep 10 '14 at 13:30
  • Thanks @Deduplicator for letting me know about casting return-value of malloc. – Adarsh Sep 10 '14 at 13:34
  • Here is an interesting thing. How do allocate memory for *tempQuestion* without using sizeof operator @Deduplicator. – Adarsh Sep 10 '14 at 13:35
  • Hm, I don't think I said not to use `sizeof`. Anyway, use the `sizeof`-operator thus: `T* temp = malloc(sizeof*temp);` (for any type `T`). – Deduplicator Sep 10 '14 at 13:40
  • How would you allocate memory that way, as the structure Question has lot many pointers as the member. Firstly you have to allocate memory to *testQuestion* right, once you done allocating memory to it, then you can probably use it. – Adarsh Sep 10 '14 at 13:44
  • Be aware that, unless you ask about a VLA, `sizeof` returns a compile-time-constant and the argument is unevaluated. – Deduplicator Sep 10 '14 at 13:52
0

At the end of AddQuestion() tempQuestion points to the same address that exam.phead points to, so CleanUp() frees that memory for you.

JoKo
  • 142
  • 1
  • 7