Run-time check: stack around variable was corrupted

Question

I am creating a program that rewrites an array with values from a file. I have linked the code below. When running the file I get the error "Run-time check failure, stack around 'arr' variable was corrupted.

Also, the output of the program returns all the array locations with the same number,

arr[0] = -858993460

The numbers in the file, separated by a line are:
12
13
15

#include<iostream>;
#include<fstream>;
using namespace std;
template <class T>
void init(T * a, int size, T v)//done
{
    for (int i = 0; i < size; i++)
    {
        a[size] = v;
    }
}

bool getNumbers(char * file, int * a, int & size)//not done
{
    int count = 0;
    ifstream f(file);

    while (f.good() == true && count < 1)
    {
        f >> a[count];
        count++;
    }

    if (size > count)
    {
        return true;
    }
    else if (size < count)
    {
        return false;
    }
}

void testGetNumbers()
{
    int arr[5];
    int size = 5;

    cout << "Testing init and getNumbers." << endl;

    init(arr, 5, -1);

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

    if (getNumbers("nums.txt", arr, size))
    {
        cout << size << " numbers read from file" << endl;
    }
    else
    {
        cout << "Array not large enough" << endl;
    }

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

    cout << endl;
}

int main()
{
    testGetNumbers();
    return 0;
}

Answer 1

Starting with the main function, the line

return 0;

... is not necessary because that's the default return value for main. I would remove it, some people insist on having it, I think most people don't care. But it's always a good idea to be fully aware of what the code expresses, implicitly or explicitly, so: returning 0 expresses that the program succeeded.

For an explicit main return value I recommend using the names EXIT_SUCCESS and EXIT_FAILURE from the <stdlib.h> header.

Then it's much more clear.

---

main calls testGetNumbers, which, except for an output statement, starts like this:

int arr[5];
int size = 5;
init(arr, 5, -1);

As it happens the init function is has Undefined Behavior and doesn't fill the array with -1 values as intended, but disregard. For now, look only at the verbosity above. Consider writing this instead:

vector<int> arr( 5, -1 );

Using std::vector from the <vector> header.

---

Following the call chain down into init, one finds

a[size] = v;

That attempts to assign value v to the item just beyond the end of the array.

That has Undefined Behavior.

Should probably be

a[i] = v;

But as noted, this whole function is redundant when you use std::vector, as you should unless strictly forbidden by your teacher.

---

Back up in testGetNumbers it proceeds to call getNumbers, in that function we find

ifstream f(file);

while (f.good() == true && count < 1)
{
    f >> a[count];
    count++;
}

Generally one should never use f.good() or f.eof() in a loop condition: use f.fail(). Also, ~never compare a boolean to true or false, just use it directly. Then the loop can look like this:

ifstream f(file);

while (!f.fail() && count < 1)
{
    f >> a[count];
    count++;
}

Tip: in standard C++ you can write ! as not and && as and. With the Visual C++ compiler you have to include the <iso646.h> header to do that.

Disclaimer: the fixes noted here do not guarantee that the loop is correct for your intended purpose. Indeed the increment of count also when the input operation fails, looks probably unintended. Ditto for the loop limit.

---

The function continues (or rather, ends) with

if (size > count)
{
    return true;
}
else if (size < count)
{
    return false;
}

This has a big problem: what if size == count? Then the execution continues to fall off the end of the function without returning a value. This is again Undefined Behavior.

I leave it to you to decide what you want the function to return in that case, and ensure that it does that.

Answer 2

In your init function...

template <class T>
void init(T * a, int size, T v)//done
{
    for (int i = 0; i < size; i++)
    {
        a[size] = v;
    }
}

Nope:

a[size] = v;

Yup:

a[i] = v;

Answer 3

This line in the first loop looks like having error.

a[size] = v;

It causes out of array bound access and memory/stack corruption. It should be

a[i] = v;