C function stops after few iterations

Question

The program should do this: write a doubleArray() function, which takes in input an array of int and its size (as a pointer to int). In the main(): ask the user to input an integer n between 1 and 4, then dynamically create an array of size n. Then start filling the array with 2048 randomly generated int: each time the array is full, call the doubleArray function; each time the function doubleArray is called, print the content of the array.

My code works until the size of array n reach a number around 250, then stops inside the for loop.

#include<stdlib.h>
#include<stdio.h>
#include<time.h>
void doubleArray(int vect[], int *dim)
{
    int n = *dim *2;
    *dim = n;
    vect = (int*)realloc(vect, n*sizeof(int));
}

void stampaArray(int vect[], int dim)
{
    for (int i=0;i<dim;i++)
    {
        printf("%d ",vect[i]);
    }
    printf("\n");
}

int main()
{
    printf("Insert a number between 1 and 4: ");
        int n;
    scanf("%d",&n);
    if ((n<1)||(n>4))
    {
        printf("Number not valid, try again: '");
        scanf("%d",&n); 
    }
    int *arr = (int*) malloc (n*sizeof(int));
    srand(time(NULL));
    int num;
    for (int i=0;i<220;i++)
    {
        num = rand();
        if (i==n)
        {
            doubleArray(arr, &n);
            stampaArray(arr, n);
        }
        arr[i]=num;
    }
    stampaArray(arr,n);
    return 0;
}

Your input validation is incorrect. You ask for `n` once, if invalid - you ask again. Then not checking anymore. Also it looks like you do not expect `n` to be greater than `4`, so why `250`? — Eugene Sh., Dec 19 '18 at 16:40
Can you elaborate `each time the array is full ` as `if(i==n)` part ? — Achal, Dec 19 '18 at 17:09

Achal · Answer 1 · 2018-12-19T16:50:18.163

Firstly, Change this

if ((n<1)||(n>4)) { } /* use && instead of || to scan if n if both condition are true*/

to

//scanf("%d",&n); /*remove this, use only once, in below loop */
while(1) {
    scanf("%d",&n);
    if ((n>=1) && (n<=4)) {
       break; 
    }
    else {
       printf("Number not valid, try again: '");
    }
}

And allocate memory equal to n bytes. for e.g

int *arr = malloc (n * sizeof(*arr)); /* typecasting is not required */

Also here

for (int i=0;i<220;i++) { /* some code */ }

what is the rationale behind rotating loop 220 times, doesn't it should be n times ?

score 0 · Answer 2 · answered Dec 19 '18 at 17:24

As you were said in comment, your main error is that realloc is allowed to change the pointer value. If it happens, the new value is only assigned to the local copy inside the doubleArray function, but the caller still keeps the previous value which is now a dangling pointer (pointing to non allocated memory). Using it invokes Undefined Behaviour (and crashes are to be expected...)

The correct way is to return the new pointer value:

int * doubleArray(int vect[], int *dim)
{
    int n = *dim *2;
    *dim = n;
    return realloc(vect, n*sizeof(int));
}

That is not all. best practices recommend to test allocation. In a stressed environment, the system could be unable to allocate enough memory and realloc could return NULL. Proceeding would then also involve Undefined Behaviour.

Let us go on. Controlling input is nice, but a user can type twice an error, so you should loop until you get a correct value:

int n;
for (;;) {
    printf("Insert a number between 1 and 4: ");
    scanf("%d",&n);
    if ((n >= 1) && (n <= 4)) break;
    printf("Number not valid, try again: '");
}

And please, please do not cast malloc in C language. It is useless and can hide hard to find indirection level errors.

Finally, I cannot understand why you have a loop up to 220... From your requirements it should be up to 2048.

Last point (but this one is only my opinion, not a problem): I would only display the initialized content of the array, so up to i instead of n. That way you would see the array grow while always keeping the same (initialized) values:

int *arr = malloc (n*sizeof(int));
srand(time(NULL));
int num;
for (int i=0;i<2048;i++)
{
    num = rand();
    if (i==n)
    {
        arr = doubleArray(arr, &n);
        if (arr == NULL) {
            perror("allocation error");
            return 1;
        }
        stampaArray(arr, i);
        printf("\n");
    }
    arr[i]=num;
}
stampaArray(arr,2048);
free(arr); // not required immediately before a return but good practice

C function stops after few iterations

2 Answers2