tags:

views:

78

answers:

7

I'm trying to create a function that would dynamically allocate an array, sets the values of the elements, and returns the size of the array. The array variable is a pointer that is declared outside the function and passed as a parameter. Here is the code:

#include <cstdlib>  
#include <iostream>  
using namespace std;  

int doArray(int *arr) {  
    int sz = 10;  
    arr = (int*) malloc(sizeof(int) * sz);  

    for (int i=0; i<sz; i++) {  
        arr[i] = i * 5;  
    }  

    return sz;  
}  

int main(int argc, char *argv[]) {  

    int *arr = NULL;  
    int size = doArray(arr);  

    for (int i=0; i<size; i++) {  
        cout << arr[i] << endl;  
    }  

    return 0;  

}  

For some reason, the program terminates on the first iteration of the for loop in main()! Am I doing something wrong?

+2  A: 

If you want to allocate memory that way you have to use:

int doArray(int*& arr)

else the pointer will only be changed inside the function scope.

caahab
A: 

The arr variable in your function is a local copy of the arr pointer in the main function, and the original is not updated. You need to pass a pointer-to-pointer or pointer reference (the former will also work in plain c, the later only in c++).

int doArray(int **arr)

or

int doArray(int*& arr)
dmckee
+1  A: 

You need to add an extra level of indirection to doArray. As written it allocates the array properly but it doesn't communicate the pointer value back to the caller correctly. The pointer from malloc is lost once you return.

If you wrote a function to take a float and change the value, passing the changed value back to the caller, it would need to take a pointer: foo(float *f). Similarly, here you want to pass back an int* value to the caller so your function must be declared as doArray(int **arr) with a second asterisk.

int doArray(int **arr) {  
    int sz = 10;  
    *arr = (int*) malloc(sizeof(int) * sz);  

    for (int i=0; i<sz; i++) {  
        (*arr)[i] = i * 5;  
    }  

    return sz;  
}  

int main(int argc, char *argv[]) {  

    int *arr = NULL;  
    int size = doArray(&arr);  

    for (int i=0; i<size; i++) {  
        cout << arr[i] << endl;  
    }  

    return 0;  

}

Notice how it now dereferences *arr inside of doArray, and how the call is now written as doArray(&arr).

John Kugelman
A: 

Change signature to (specific for c++):

int doArray(int *&arr)

so pointer would be changed at exit from doArray.

Dewfy
A: 

You need a pointer to a pointer in your doArray() parameter. If you've never done any programming with pointers before, this can be confusing. I find that it can be easier to see the right types if you annotate your code abundantly with typedefs.

You have the right idea that (int*) can be used to represent an array. But if you want to change the value of your variable arr in main(), you need a pointer to that, and so you will end up with (untested code) something like the following

typedef int *IntArray;

int doArray(IntArray *arr) {  
    int sz = 10;  
    *arr = (IntArray) malloc(sizeof(int) * sz);  
    IntArray theArray = *arr;

    for (int i=0; i<sz; i++) {  
        theArray[i] = i * 5;  
    }  

    return sz;  
}

when calling doArray, you will need to pass the address of your variable (so doArray knows where to write to):

int main(int argc, char *argv[]) {  

    int *arr = NULL;  
    int size = doArray(&arr);  

    for (int i=0; i<size; i++) {  
        cout << arr[i] << endl;  
    }  

    return 0;  

}  

This should work.

Carlos Scheidegger
+1  A: 

You're passing in the array pointer by value; this means that when your doArray function returns, the value in arr in main is still NULL - the assignment inside doArray doesn't change it.

If you want to change the value of arr (which is an int *), you need to pass in either a pointer or a reference to it; hence, your function signature will contain either (int *&arr) or (int **arr). If you pass it in as a ** you'll also have to change the syntax inside the function from using arr to *arr (pointer-dereferencing), and you'll call it like so: doArray(&arr).

Also, in C++ you should really be using new int[sz] instead of malloc.

tzaman
A: 

As others have pointed out, you're passing your array (the int *) by value, so when you say arr=... you're not actually changing the array you passed in.

You're also got a memory leak, as you've written it. It's not a big deal when you only call doArray once in the body of your program, but if it gets called repeatedly and the array is never freed (or deleted, if you made it with new) then it can cause problems. Typically the best way to deal with this is by using the STL. You'd then write

#include <vector>
#include <iostream>
int doArray(std::vector<int> &arr) {  
    int sz = 10;
    arr.resize(sz);  
    for (int i=0; i<sz; i++) {  
        arr[i] = i * 5;  
    }
    return sz;  
}

int main(int argc, char *argv[]) {  
    std::vector<int> arr;
    int size = doArray(arr);  
    for (int i=0; i<size; i++) {  
        std::cout << arr[i] << std::endl;  
    }
    return 0;  
}

However, with the STL there are more idomatic ways than returning the size, since you can just ask for arr.size(), and if you get really fancy, can use functions like for_each or the ostream_iterator to print all your elements.

Steve