samprat
samprat

Reputation: 2214

What's wrong with the following code?

int *intialize(void) 
{
    int value[64];

    for ( int i = 0; i < 64 ; i++)
    {
        value[i] = i;

       return value;   
    }

}  

int main( )
{
    int * p;

    p = intialize();
    p[32] = 100;
    printf("%d", p[32]);

    return 0;
}

Above is the sample code that was asked to me in an interview. Though I failed to find error and obviously rejected, I am curious to know what is exactly wrong with this code.

Upvotes: 2

Views: 451

Answers (2)

user418748
user418748

Reputation:

int value[64];  

for ( int i = 0; i < 64 ; i++)  
{
    value[i] = i;  
}
return value;

value is defined in the local scope of initialize() and also, if you could assume that was valid, you return the memory location on the first iteration, hence making the contents of value[] garbage after value[0].

When you define a variable in the local scope, the variable ceases to exist when the function reaches termination. Returning a pointer to a local variable invokes Undefined Behaviour cause you access (and use) memory you should not.

Undefined Behaviour mate ;)


If you want to make it correct you should do something like :

int  * result = malloc(sizeof(int)* 64);  
if(!result)
    return 0;

for ( int i = 0; i < 64 ; i++)  
{
    result[i] = i;  
}
return result;

And also check in your main() if initialize() returns 0 or not (AKA if malloc() succeeded or Failed) and if the return value was not 0 make sure you free() the memory.

int main( )
{
    int * p;

    p = intialize();
    if(p)
    {
         p[32] = 100;
         printf("%d", p[32]);
         free(p);
    }
    return 0;
}

You could also have the free() outside the if clause, cause doing a free(0) is safe.

Upvotes: 10

Snips
Snips

Reputation: 6763

'intialize' is a typo (though it is a consistent typo, so will compile).

Did I get it right?

Upvotes: 2

Related Questions