Should I use both 'if' and 'else' conditions when dynamically allocating memory in C?

Question

Consider:

int main() {

    int* index = (int*)malloc(n*sizeof(int));

    if(index == NULL) {
        printf("Memory Not Allocated.\n");
        exit(0);
    }

    int* arr = (int*)malloc(p*sizeof(int));

    if(arr == NULL) {
        printf("Memory Not Allocated.\n");
        exit(0);
    }

    int* queue = (int*)malloc(frames*sizeof(int));

    if(queue == NULL) {
        printf("Memory Not Allocated.\n");
        exit(0);
    }

    // More code...
    // ---------------
    // ---------------
    // More code....

}

This is a part of my program. In this code I have dynamically allocated memory to index, arr and queue. Also I have used if condition for check weather the memory is allocated or not. I only used if part. I didn't used else part because I have used exit(0) inside of if condition. Is this way correct? or should I also use else part like below?

int main() {

    int* index = (int*)malloc(n*sizeof(int));

    if(index == NULL) {
        printf("Memory Not Allocated.\n");
        exit(0);
    }
    else {

        int* arr = (int*)malloc(p*sizeof(int));

        if(arr == NULL) {
            printf("Memory Not Allocated.\n");
            exit(0);
        }
        else {
            int* queue = (int*)malloc(frames*sizeof(int));

            if(queue == NULL) {
                 printf("Memory Not Allocated.\n");
                 exit(0);
            }
            else {

                 // More code...
                 // ---------------
                 // ---------------
                 // More code....
            }
        }
    }

As I suppose, the program will exit if the memory is not allocated because of exit(0). Therefore, it's not necessary to use the else part. That's my opinion.

What is the correct and better method? Are these both ways are okay to use?

Answer 1

As I suppose program will exit if the memory is not allocated because of exit(0). Therefore, it's not necessary to use else part.

Yes, that is correct. The else is not necessary, and using else makes the code harder to read. So it should not be used.

The method I would use is shown below:

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

int main(void) 
{
    int *index = malloc(n*sizeof(int));
    int *arr   = malloc(p*sizeof(int));
    int *queue = malloc(frames*sizeof(int));

    if (index == NULL || arr == NULL || queue == NULL) {
        printf("Memory Not Allocated.\n");
        free(index);
        free(arr);
        free(queue);
        exit(EXIT_FAILURE);
    }

    more codes...
    ---------------
    ---------------
    more codes....
}

Things to note:

1. In C, you should not cast the return value from malloc. See this question for more information.
2. You can defer the error checking until all of the memory is allocated. The expectation is that all of the allocations will succeed. In the unlikely event that one or more allocations fail, the code will waste a little time before giving the user the bad news. But otherwise there's no harm in trying all of the allocations. The user is unlikely to notice the extra time, and will be much more concerned about the fact that the program didn't run.
3. After attempting to allocate all of the memory, you can use a single if to check whether all of the allocations succeeded.
4. Passing a NULL pointer to free is specifically allowed by the C standard, so as long as you've initialized each pointer (either with a valid address or NULL), you can pass each pointer to free (without bothering to check if it's NULL).
5. exit(0) is used to inform the shell that the program succeeded. Calling exit with a non-zero value (typically 1) indicates failure. The macro EXIT_FAILURE from <stdlib.h> is the official non-zero value that should be passed to exit to indicate failure.

Answer 2

Unless you are writing a trivial program, one should avoid stand-alone bare-bones mallocs on the top-level of your program.

Now it may seem that I am about to go beyond the scope of your question, but in fact, your question begs the broader answer, because, to do our best programming, we must always be aware how our sharp, narrowly focused concern fits in the larger scheme of things, and that is what I wish to address.

The first concern when dealing with mallocs are memory leaks. You did a great job for the first part of that problem by making sure that on failure they got deallocated. Just be aware that there are several additional concerns down the road.

The biggest and most frequent problems with use of mallocs is that it is very easy to end up with memory leaks. Again, I can't emphasize this too much. This should be the number one concern every time you use mallocs.

Compared to this number one issue, branching style, as long as it is structured, and you have made some effort to be clean and direct, is of secondary importance to handling of memory leaks (e.g., an English accent is just as good as an Irish accent and speaking well is a bit more important, but it is much less important than whether you exhibit criminal behavior.)

There are many good ways to deal with good malloc writing.

For example, as long as you are not dealing with interrupt events or other advanced error handling concerns, let’s look at a good way to work with mallocs.

1. Make your programs strictly structured.
2. Write allocation procedures that do all the right things for malloc in pairs. One procedure to allocate memory for a certain set of scenarios, and another procedure that will deallocate exactly the same memory for the same set of scenarios in exactly a reverse fashion. These can be written in many ways. But again that the job gets done perfectly is the most important. Once you debug the procedures, you don't have to worry about whether it was done correctly because you will know. That little piece of knowledge is invaluable to how you use the procedures. You want to be able to count on them.
3. Topologically in your program, make sure that at any point where memory is allocated, that every path of execution after the allocation will at some point execute the corresponding deallocation procedure regardless of all conditionals and loops.
4. Now you written a good set of procedures you can pull them out to apply to the more advanced aspects of mallocing down the road.

You want to design these paired procedures well enough that you know for a fact that after properly executing both of them, that no memory associated with the pair of transactions is still allocated. Again style has some importance, but there are many ways to write these. What is really important is the salient fact of making sure there aren't any memory leaks. That is the most important.

Here is a nice little story. Once I was on a contract that when I came on the scene a serious memory leak problem had been around a few months which was not getting fixed. I used the methods that I described to you and within the day I went through the whole program and structured all the mallocs.

A couple of days later, the problem happened again. I knew that could not be, because I had checked all the structured paths through the program, but there it was. Because I was able to know that it was correct because of the systematic application of the simple principles of pairing the mallocs, and following the topological flow of the program, I knew it made sense to look elsewhere. After some analysis, I decided the only possibility is that there were compound interrupts touching the same memory locations which were destroying the structuring of the program. So I set a few breakpoints that would get triggered if the problem occurred, and sure enough I got a hit.

I looked at the stack and sure enough, one interrupt fudged the memory allocation and the other stepped on it. Having identified the two events, I just added a mutual exclusion between the two events. This took about 10 minutes from beginning to finish and fixed several months worth of problems.

There isn't any way that I could have found the problem without doing my mallocs in the right way in a systematic fashion. And BTW, I don't have any idea what style I used with the conditionals, and that didn't really matter much. Did it?

Answer 3

The "else" is excess in the three cases. Indeed, since there is no dependency between them, creating a decision tree is slightly confusing. A minor detail. Compare NULL == variable rather than (variable == NULL) because when we make the keystroke error of "=" instead of the correct "==". Finally, make every error message unique! In larger programs, this greatly aids debug and tech support.