8

I'm trying to declare a pointer and pass that pointer to a function where memory is allocated. Here is a minimal example:

#include <string>
#include <iostream>

using namespace std;

void alloc_mem(int &size, double *x);

int main()
{

        double *X;
        int imax;

        alloc_mem(imax, X);

        cout << "imax = " << imax << endl;
        for (int i = 0; i < imax; i++) {
                cout << "X = " << X[i] << endl;
        }

        delete[]X;
        return 0;

}

void alloc_mem(int &size, double *x)
{

        size = 10;
        x = new double[size];
        for (int i = 0; i < size; i++) {
                x[i] = (double)i;
        }

}

This code compiles, but I get a segmentation fault when I try to print out the values of X. I know that I'm not passing the variable into the function correctly, but I'm not sure how to do it. I believe that I'm operating on a copy of x.

Also, this code was written to reproduce an issue I'm having in a much larger code.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
James
  • 167
  • 2
  • 2
  • 6
  • 4
    Try this prototype : `void alloc_mem(int &size, double*& x);`. else x is modified only in the function. – Jarod42 Apr 18 '14 at 15:17
  • You are *declaring* the pointer, **not** *initializing* it. – Chris Stratton Apr 18 '14 at 15:20
  • 1
    I would use `std::vector` instead. – Ivan Apr 18 '14 at 15:34
  • The prototype mentioned in the first comment works. I swear that I tried that at 2:00 am last night. Thanks! – James Apr 18 '14 at 15:35
  • Why would you like to implement such function as the one above (besides the curiosity)? This approach is very unsafe. I'd listen to Ivan and use std::vector<> instead. – Adam Wulkiewicz Apr 18 '14 at 16:44
  • You are right that I should use std::vector<>, but out of curiosity, why is it very unsafe? For every new, I have a delete. I'm watching for memory leaks and will run my entire code through a debugger to check. – James Apr 20 '14 at 03:34
  • Update: You all are right, std::vector<> is better/safer. I converted the entire code to use vectors instead of dynamically allocated arrays. – James Apr 23 '14 at 23:28

3 Answers3

19

Parameter double *xis a local variable of function alloc_mem. When the function will end its execution the variable will be destroyed. The original variable X in main knows nothing what was done with this parameter because it was passed by value that is a copy of it was used in the function.

Either pass the pointer by pointer or by reference. For example

void alloc_mem(int &size, double **x);

void alloc_mem(int &size, double * &x);

void alloc_mem(int &size, double **x) 
{
   size = 10;

   *x = new double [size];

   for ( int i = 0; i < size; i++ ) ( *x )[i] = i;
}

void alloc_mem(int &size, double * &x) 
{
   size = 10;

   x = new double [size];

   for ( int i = 0; i < size; i++ ) x[i] = i;
}

As for me I would define the function the following way

double * alloc_mem( int &size ) 
{
   size = 10;

   x = new double [size];

   for ( int i = 0; i < size; i++ ) x[i] = i;

   return x;
}

if size is known before calling the function then it could be written even simpler

double * alloc_mem( int size ) 
{
   x = new double [size];

   for ( int i = 0; i < size; i++ ) x[i] = i;

   return x;
}

Take into account that loop

   for ( int i = 0; i < size; i++ ) x[i] = i;

can be substituted for standard algorithm std::iota For example

std::iota( x, x + size, 0.0 );
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
4

The standard C++ mechanism for defining an allocation function is operator new.

That's why the standard calls it an allocation function.

Note that operator new is different from a new-expression.

A new-expression uses the relevant allocation function (operator new function) to allocate memory, and the relevant constructor to then initialize.

However, in your case you're using (your named) allocation function just to allocate and initialize a dynamic array. Allocation and initialization are well separated responsibilities in the standard C++ language design, for very good reasons, and it would be a good idea to follow that convention. Use std::vector for your array, and if you really really really need custom allocation (very doubtful that you do), then use a custom allocator for that std::vector.


Concrete example.

Replace your current code

int main () { 

// Declaring variables
double* X;
int imax;

// Calling function
alloc_mem(imax,X);

// Printing
cout << "imax = " << imax << endl;
for (int i=0; i<imax; i++) {
    cout << "X = " << X[i] << endl;
}

with

#include <vector>

int main() {
    int const imax = whatever();
    std::vector<double> X( imax );

    cout << "imax = " << imax << endl;
    for (int i=0; i<imax; i++) {
       X[i] = i;  // If you really want these values in the vector.
       cout << "X = " << X[i] << endl;
    }
}
Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • I use an iterative approach to writing answers. What with eyesight and incredibly bad laptop keyboards, not to mention unreliable and sometimes snail's pace connection, that's the only practical way for me. :( But thanks! – Cheers and hth. - Alf Apr 18 '14 at 15:27
  • _'I use an iterative approach to writing answers'_ I do so as well, usually I start out with writing a comment, notice it would be a suitable answer, copy it to the answer editor and post the 1st shot. Afterwards I continue editing, and usually don't expect anyone to correct my post meanwhile ;) ... – πάντα ῥεῖ Apr 18 '14 at 15:31
3

When you have an output parameter, you can pass it by reference, or by pointer.

For your size parameter, you passed by reference (since it's an out parameter).

The other parameter is a double*, so either add a reference:

void alloc_mem(int & size, double* & x)  // (1)

or add another pointer (i.e. another level of indirection):

void alloc_mem(int & size, double** x)   // (2)

For coherence, since you used the reference (&) technique for size, I'd suggest to use it for x as well (as in (1)).

Note also that in C++ you may want to just pass a std::vector, which knows its own size, and does automatic cleanup (thanks to its destructor):

void alloc_mem(std::vector<double> & x)

Note also that you may want to return the vector as a return value as well (simplifying your code):

std::vector<double> alloc_mem()

Code snippets:

// Note: the caller must free the memory using delete[].
void alloc_mem(int& size, double*& x) {
    size = 10;
    x = new double[size];
    for (int i = 0; i < size; i++) {
        x[i] = i;
    }
}

// Note: automatic vector cleanup. No need of manual delete.
std::vector<double> alloc_mem() {
    const int size = 10;
    std::vector<double> x(size); // size is 0
    for (int i = 0; i < size; i++) {
        x[i] = i;
    }
    return x;    
}
Mr.C64
  • 41,637
  • 14
  • 86
  • 162