CODEWITHSUNDEEP
c++memorymalloc
Roronoa Zoro
Roronoa Zoro

Reputation: 1013

Why was malloc used this way?

I'm working on a code I requested from a researching team. I'm trying to understand the code, however, they used malloc in a strange way. here;

in the header file;

 #define ERROR_OUTPUT stderr
 #define FAILIF(b) {   \
   if (b) {  \
       fprintf(ERROR_OUTPUT, "FAILIF triggered on line %d, file %s. Memory allocated: %lld\n",  \
       __LINE__, __FILE__, totalAllocatedMemory); exit(1); \
   } \
  }
 #define MALLOC(amount) \ 
   ( (amount > 0) ? totalAllocatedMemory += amount, malloc(amount) : NULL)

in the cpp file;

 double *memRatiosForNNStructs = NULL;
 double *listOfRadii = NULL;
 nRadii = 1;
 FAILIF(NULL == (memRatiosForNNStructs = (double*)MALLOC(nRadii * sizeof(double))));

From my understanding, the MALLOC that they defined means the following;

if(amount>0) // which is true; at least in this case
{
   totalAllocatedMemory = totalAllocatedMemory + amount; // sounds reasonable
   malloc(amount)  // What?? This will leak memory...
}
else
{
   NULL; // Do nothing? I guess that's fine
}

Is there something that I'm missing here? Or did they just make a (naive) mistake?

Upvotes: 1

Views: 353

Answers (4)

In silico
In silico

Reputation: 52159

The third code snippet you have is not equivalent. Notice the use of the comma operator:

#define MALLOC(amount) \
    ( (amount > 0) ? totalAllocatedMemory += amount, malloc(amount) : NULL)  
                                                   ^
                                                  N.B.!

The comma operator takes two arguments, evaluates and discards the first expression, then evaluates and returns the second expression.

The ternary conditional operator used this way

a = ((b)?(c:d))

is equivalent to this

if(b) {
    a = c;
}
else {
    a = d;
}

The comma operator used this way

e = f, g;

is equivalent to this

f;
e = g;

So if you have

a = ((b)?(f,g:d))

then that is equivalent to

if(b) {
    f;
    a = g;
}
else {
    a = d;
}

In the code provided in your original post, the MALLOC macro is to be used like this:

memRatiosForNNStructs = (double*)MALLOC(nRadii * sizeof(double));

which is equivalent to this:

   // First operand of ternary conditional
if(nRadii * sizeof(double) > 0)
{
    // Second  operand of ternary conditional

    // First expression of the comma operator
    totalAllocatedMemory += nRadii * sizeof(double));
    // Second expression of the comma operator
    memRatiosForNNStructs = (double*)malloc(nRadii * sizeof(double));
}
else
{
    // Third operand of ternary conditional
    memRatiosForNNStructs = (double*)NULL;
}

Honestly though, this could be achieved as a function in C with no loss of generality:

void* my_malloc(unsigned int amount)
{
    if(amount > 0) {
        // I assume this is a global variable
        totalAllocatedMemory = totalAllocatedMemory + amount;
        return  malloc(amount);
    }
    else {
        return NULL;
    } 
}

memRatiosForNNStructs = (double*)my_malloc(nRadii * sizeof(double));

So I'm not sure what's the point of even implementing it as a hard-to-read macro.

Upvotes: 8

Kerrek SB
Kerrek SB

Reputation: 477060

You're dealing with the comma operator, which evaluates each operand in order and returns the return value of the last operand. So a + b, malloc(n) first evaluates a + b, then malloc(n), and then returns the result of the latter. So the return type of the entire ternary conditional expression is void *.

The best approximation to the ternary expression is a function:

void * MALLOC(unsigned int n) {
   if (n > 0) {
      totalAllocatedMemory += n;
      return malloc(n);
   } else {
      return NULL;
   }
}

Upvotes: 3

Andrew Cooper
Andrew Cooper

Reputation: 32576

The MALLOC macro is not quite the same as your expansion.

The ternary operator will return the pointer returned by the call to malloc in the case that the condition is true, and a NULL pointer in the case that the condition is false.

As long as you assign the result of the ternary operator to a vairable there's no memeory leak.

Upvotes: 1

Matteo Italia
Matteo Italia

Reputation: 126797

They are using a confusing combination of the ternary operator and of the comma operator.

Keep in mind that:

  • the ternary operator evaluates its first operand and returns the second if the first is true, otherwise the third;
  • the comma operator evaluates both its operands and returns the second.

Thus, if amount>0, the expression will return totalAllocatedMemory += amount, malloc(amount); since the comma operator executes both expressions but returns only the second, the final return value of the expression is the one from the malloc.

Still, that ugly macro would have been much clearer (and with no performance loss) if it were an inline function.

Upvotes: 1

Related Questions