Function receives a pointer to double, allocates memory and fills resulted array of doubles

Question

My goal is to pass a pointer to double to a function, dynamically allocate memory inside of the function, fill resulted array with double values and return filled array. After lurking attentively everywhere in StackOverflow, I have found two related topics, namely Initializing a pointer in a separate function in C and C dynamically growing array. Accordingly, I have tried to write my own code. However, the result was not the same as it was described in aforementioned topics. This program was run using both gcc and Visual Studio.

First trial.

int main()
{
    double *p;
    int count = getArray(&p);
    <...print content of p...>
    return 0;
}

int getArray(double *p)
{
    int count = 1;
    while(1)
    {
        if(count == 1)
            p = (double*)malloc(sizeof(double));
        else 
            p = (double*)realloc(p, count*sizeof(double));
        scanf("%lf", &p[count-1]);
        <...some condition to break...>
        count++;
    {
    <... print the content of p ...>
    return count;
}

(Here comes the warning from compiler about incompatible argument type. Ignore it).

Input:

1.11
2.22
3.33

Output:

1.11
2.22
3.33

0.00
0.00
0.00

Second trial.

int main()
{
    double *p;
    int count = getArray(&p);
    <...print content of p...>
    return 0;
}

int getArray(double **p)
{
    int count = 1;
    while(1)
    {
        if(count == 1)
            *p = (double*)malloc(sizeof(double));
        else 
        {
            double ** temp = (double*)realloc(*p, count*sizeof(double));
            p = temp;
        }
        scanf("%lf", &(*p)[count-1]);
        <...some condition to break...>
        count++;
    {
    <... print the content of p ...>
    return count;
}

Input:

1.11
2.22
Segmentation error.

I tried this method on several different *nix machines, it fails when the loop uses realloc. SURPRISINGLY, this code works perfect using Visual Studio.

My questions are: first code allows to allocate and reallocate the memory and even passes all this allocated memory to main(), however, all the values are zeroed. What is the problem? As for the second program, what is the reason of the segmentation error?

Answer 1

The right way of doing it is like this:

int getArray(double **p)
{
    int count = 0;
    while(1)
    {
        if(count == 0)
            *p = malloc(sizeof(**p));
        else 
            *p = realloc(*p, (count+1)*sizeof(**p));
        scanf("%lf", &((*p)[count]));
        <...some condition to break...>
        count++;
    {
    <...print content of p...>
    return count;
}

If you pass a pointer to a function and you want to change not only the value it is pointing at, but change the address it is pointing to you HAVE to use a double pointer. It is simply not possible otherwise.

And save yourself some trouble by using sizeof(var) instead of sizeof(type). If you write int *p; p = malloc(sizeof(int));, then you are writing the same thing (int) twice, which means that you can mess things up if they don't match, which is exactly what happened to you. This also makes it harder to change the code afterwards, because you need to change at multiple places. If you instead write int *p; p = malloc(sizeof(*p)); that risk is gone.

Plus, don't cast malloc. It's completely unnecessary.

One more thing you always should do when allocating (and reallocating) is to check if the allocation was successful. Like this:

if(count == 0) 
    *p = malloc(sizeof(**p));
else 
    *p = realloc(*p, (count+1)*sizeof(**p));

if(!p) { /* Handle error */ }

Also note that it is possible to reallocate a NULL pointer, so in this case the malloc is not necessary. Just use the realloc call only without the if statement. One thing worth mentioning is that if you want to be able to continue execution if the realloc fails, you should NOT assign p to the return value. If realloc fails, you will lose whatever you had before. Do like this instead:

int getArray(double **p)
{
    int count = 0;

    // If *p is not pointing to allocated memory or NULL, the behavior
    // of realloc will be undefined.
    *p = NULL;

    while(1)
    {
        void *tmp = realloc(*p, (count+1)*sizeof(**p));
        if(!tmp) {
            fprintf(stderr, "Fail allocating");
            exit(EXIT_FAILURE);
        }

        *p = tmp;

        // I prefer fgets and sscanf. Makes it easier to avoid problems
        // with remaining characters in stdin and it makes debugging easier

        const size_t str_size = 100;
        char str[str_size];
        if(! fgets(str, str_size, stdin)) {
            fprintf(stderr, "Fail reading");
            exit(EXIT_FAILURE);
        }

        if(sscanf(str, "%lf", &((*p)[count])) != 1) {
            fprintf(stderr, "Fail converting");
            exit(EXIT_FAILURE);
        }

        count++;

        // Just an arbitrary exit condition
        if ((*p)[count-1] < 1) {
            printf("%lf\n", (*p)[count]);
            break;
        }
    }
    return count;
}

You mentioned in comments below that you're having troubles with pointers in general. That's not unusual. It can be a bit tricky, and it takes some practice to get used to it. My best advice is to learn what * and & really means and really think things through. * is the dereference operator, so *p is the value that exists at address p. **p is the value that exists at address *p. The address operator & is kind of an inverse to *, so *&x is the same as x. Also remember that the [] operator used for indexing is just syntactic sugar. It works like this: p[5] translates to *(p+5), which has the funny effect that p[5] is the same as 5[p].

In my first version of above code, I used p = tmp instead of *p = tmp and when I constructed a complete example to find that bug, I also used *p[count] instead of (*p)[count]. Sorry about that, but it does emphasize my point. When dealing with pointers, and especially pointers to pointers, REALLY think about what you're writing. *p[count] is equivalent to *(*(p+count)) while (*p)[count] is equivalent to *((*p) + count) which is something completely different, and unfortunately, none of these mistakes was caught even though I compiled with -Wall -Wextra -std=c18 -pedantic-errors.

You mentioned in comments below that you need to cast the result of realloc. That probably means that you're using a C++ compiler, and in that case you need to cast, and it should be (double *). In that case, change to this:

double *tmp = (double*)realloc(*p, (count+1)*sizeof(**p));
if(!tmp) {
    fprintf(stderr, "Fail allocating");
    exit(EXIT_FAILURE);
}

*p = tmp;

Note that I also changed the type of the pointer. In C, it does not matter what type of pointer tmp is, but in C++ it either has to be a double* or you would need to do another cast: *p = (double*)tmp