Segfault and uninitialized data when generating a random array in C

Question

#include "stdafx.h"
#include <stdlib.h>
#include <time.h>
#define len 10

int *randomArray(void);

int main()
{
    srand(time(NULL));
    int *rArray = (int *)malloc(sizeof(int) * len);
    rArray = randomArray();
    for (int i = 0; i < len; i++) {
        printf("%d ", *(rArray+i));
    }
    puts("");
    free(rArray);
}

int *randomArray(void)
{
    int array[len] = { 0 };
    for (int i = 0; i < len; i++) {
        array[i] = rand() % len;
    }
    return array;
}

Task is to create an array of ints and have a function fill that array with random numbers. The function randomArray() works just fine, but for some reason the assignment rArray = randomArray() doesn't work correctly, although some elements of rArray are valid numbers not gibberish. Also, the last main line free(rArray); crashes the program which is just mind numbing for me, it doesn't make any sense. If I remove the line the program doesn't crash but we all know you need to free() a malloc()-ed array.

score 1 · Accepted Answer · answered Aug 29 '18 at 07:54

The primary problem here is, array is a local variable in the randomArray() function scope. Once the function returns, the returned address becomes invalid. Any further attempt to use the memory will lead to undefined behavior.

Moreover, from your approach, you are trying to overwrite the allocated memory by the address being returned from the function call, which will cause memory leak. Rather, change your design, pass the allocated memory to the function as the argument and just fill the elements using the rand() call.

Some programmer dude · Answer 2 · 2018-08-29T07:57:39.273

1

The randomArray return a pointer to the first element of the local array array.

That pointer becomes invalid immediately once the function returns as the variable goes out of scope. Using it in any way will lead to undefined behavior.

What makes it even worse is that you reassign the pointer rArray, making you lose the original memory you allocated. That means your call to free again will lead to UB.

To solve both problems, pass the pointer and the size as arguments to the randomArray function:

void randomArray(int *array, int size)
{
    for (int i = 0; i < size; ++i)
    {
        array[i] = rand() % size;
    }
}

edited Aug 29 '18 at 07:57

answered Aug 29 '18 at 07:56

Some programmer dude

400,186
35
402
621

True, but in this case, passing size would be redundant, as it is defined as a MACRO already. Just saying. – Sourav Ghosh Aug 29 '18 at 07:57
1

@SouravGhosh When passing a pointer to an "array" it's always a good habit to pass the size of it as well. The size might not always be available as a macro. :) – Some programmer dude Aug 29 '18 at 07:58

score 0 · Answer 3 · answered Aug 29 '18 at 07:54

int *randomArray(void)
{
    int array[len] = { 0 };
    //...
    return array;
 }

array goes out of scope at } and accessing this returned pointer is UB. Moreover rArray = randomArray(); leaks memory since now you cannot free the malloc'd memory. You should pass the rArray to a function, which will be responsible for filing it.

score 0 · Answer 4 · answered Aug 29 '18 at 07:57

In randomArray() function, you are returning the array that is allocated on stack. That array will be freed when returning from randomArray() function. Instead, you can do this:

void randomArray(int * array)
{
    // Remove this int array[len] = { 0 };
    for (int i = 0; i < len; i++) {
        array[i] = rand() % len;
    }
    // Remove this .. return array;
}

And call randomArray(rArray) from main()

Segfault and uninitialized data when generating a random array in C

4 Answers4