2

I was asked to write two functions to allocate and deallocate int array in C++.

int* allocate(int size){
    return new int[size];
}

void deallocate(int *pt){
    delete pt;
    pt = NULL;
}

I came up with two functions as above.

Does anyone know is there better way to write functions to allocate/deallocate int array in C++?

gsamaras
  • 71,951
  • 46
  • 188
  • 305
1234
  • 539
  • 3
  • 12

2 Answers2

7

I doubt there is a better way

It's not about better way or not, it's about correctness.

Use

delete [] pt;

since pt is an array!

Moreover, as thorsan suggested, you set pt to NULL, but that won't be visible outside of deallocate(), see for yourself:

#include <iostream>

using namespace std;

int* allocate(int size) {
    return new int[size];
}

void deallocate(int *pt) {
    delete [] pt;
    pt = NULL;
}

int main() {
        int* pt = allocate(5);
        deallocate(pt);
        cout << (pt == NULL ? "NULL" : "not NULL") << endl;
        return 0;
}

Output:

gsamaras@pythagoras:~$ g++ -Wall main.cpp 
gsamaras@pythagoras:~$ ./a.out 
not NULL

To avoid that, simply pass a reference, like this:

void good_deallocate(int*& pt) {
        delete [] pt;
        pt = NULL;
}

You could also check if the allocation was successful in your first function, like this:

int* good_allocate(int size) {
        try {
                return new int[size];
        }
        catch(std::bad_alloc&) {
                cerr << "shit\n";
                return NULL;
        }
        // OR as Dietmar Kühl suggested
        /*
        if (int* rc = new(std::nothrow) int[size]) {
                return rc; 
        } else {
                // handle error
        }
        */
}

inspired from How to check memory allocation failures with new operator?

Community
  • 1
  • 1
gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • If you want to check whether your allocation is successful, you are much better off using the `std::nothrow` version of the allocation: `if (int* rc = new(std::nothrow) int[size]) { return rc; } else { handle_error(); }` – Dietmar Kühl May 30 '16 at 06:39
  • @DietmarKühl personally I always use what you said, I just wanted to sound more [tag:c++]-like. I will update, is it better now? :) – gsamaras May 30 '16 at 06:42
  • 2
    I don't think it is more C++-like to handle errors locally using exceptions! Although I don't mind the use of exception for non-local error handling, using them to handle errors locally seems kind of wrong. That said on systems I work on, memory allocation failures are rare and on overallocating systems don't really exist at all, i.e., I consider memory allocation failure to be truly exceptional and wouldn't handle them locally, especially as there is rarely a way to actually recover from an allocation failure locally anyway. – Dietmar Kühl May 30 '16 at 06:44
  • @DietmarKühl when experimenting with our [paper](http://arxiv.org/pdf/1603.09596.pdf), my personal computer couldn't handle the data, so I got used to it. However, *yes*, there is no way to recover (usually). – gsamaras May 30 '16 at 06:46
  • `pt == NULL` causes implementation-defined behaviour in your code, so the test may appear to succeed – M.M May 30 '16 at 07:20
  • @M.M not sure why! Should I post a question?.......Help me improve this answer then, should I replace it with `!pt`? – gsamaras May 30 '16 at 07:54
  • no, the thing is that you can't reliably use the value of a pointer after it has been freed (this is called *invalid pointer value* in the standard). For rationale, imagine a system where loading addresses into address registers checks the address against the MMU, but since you have freed the pointer that address is no longer on the accepted list. In C, and in C++ prior to C++14, it was undefined behaviour; but now it is "implementation-defined or may raise a hardware exception). – M.M May 30 '16 at 08:43
  • It is actually conforming if an implementation sets all copies (or some copies) of a freed pointer to NULL – M.M May 30 '16 at 08:44
  • Hmm I see your point @M.M, so basically what are saying is that we shouldn't check if the pointer is `NULL` after deletion? – gsamaras May 30 '16 at 09:11
  • 1
    It appears pointless here - assuming that you fix the `int*&` argument, the pointer will **always** be null afterwards. And it's quite possible that the null pointer check will just be optimized out. But technically M.M has a point; the "value" of a pointer to a deleted object is meaningless. – MSalters May 30 '16 at 09:23
0
if (int* rc = new(std::nothrow) int[size]) {
        return rc; 
} else {
        cout << "New failed, unable to allocate" << object_name << 
                ", size needed:" << sizeof( int ) * size << "." << endl;
        exit..., return... whatever, just don't use rc.
}

A lot of the comments (here and elsewhere) are about "why bother to handle?" Your program can't work, new/alloc errors don't happen on today's big (virtual) machines, etc. Think about how a program with an undetected 'new' error (or any error, for that matter) fails. For 'new', sometime later, you usually see an access violation. It may, or may not, be associated to the original failing statement/variable(s).

Now, pretend you are a user running this program (that, probably, you did not write). I usually see: "Access violation... core dumped" (or similar)? I want to see: "New failed, unable to allocate 'fluff', size = 24GB. Program terminated."

In the first example, the user calls the developer (at 2am, because these are always critical errors at worst possible time), and a major debugging effort ensues. Time to solve: hours? Days? In the second case, the user says "24GB? What the heck? Check the input - oh, oops, typo, I meant to say 24KB. Let me fix that." Time to solve: minutes. No call, no frustration.

Good diagnostics that the user can understand prevent [emergency] phone calls! Note, for example, even though I am allocating 'int's, that I included the logical object name, which hopefully means something to the user, and will aid him in fixing his problem. (Ummm, in case it need be said, emit the diagnostic at whatever level appropriate, just produce it and make it useful; it IS worth the effort.)

Matthew Miner
  • 41
  • 6