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.

7
  • 4
    Try this prototype : void alloc_mem(int &size, double*& x);. else x is modified only in the function.
    – Jarod42
    Commented Apr 18, 2014 at 15:17
  • You are declaring the pointer, not initializing it. Commented Apr 18, 2014 at 15:20
  • 1
    I would use std::vector<double> instead.
    – Ivan
    Commented Apr 18, 2014 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
    Commented Apr 18, 2014 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. Commented Apr 18, 2014 at 16:44

3 Answers 3

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 );
0
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;
    }
}
2
  • 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! Commented Apr 18, 2014 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 ;) ... Commented Apr 18, 2014 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;    
}
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.