How can I allocate the right amount of memory to this?

Question

This has somehow become susceptible to buffer overflow. Not sure why or how to fix it?

ps. I am new to programming and any tips to improve the overall quality of the code would be greatly appreciated.

#include <iostream>
#include <string>
#include <fstream>
using namespace std;

int numberSeries[] = { 1 };
int seriesSize;
int checkOrder = 1;
int counter = 0;
int i = 0;
string arr[] = { "test" };
string value;
string import;
string fileLocation;

void CreateArrayFromFile(string fileLocation);
void InputNumbers();
void PrintArray();
int Iteration(int size, int array[]);
void SwapNumbers(int arrayLocation, int array[]);

int main()
{
    cout << "What you like to numbers 'import' from file or 'manually' enter the numbers? \n";
    cin >> import;
    if (import == "import") {
        cout << "Input file location using full path: ";
        cin >> fileLocation;
        CreateArrayFromFile(fileLocation);
    }
    else if (import == "manually") {
        InputNumbers();
        PrintArray();

        while (counter < seriesSize) {
            Iteration(seriesSize, numberSeries);
        }
    }
    else {
        cout << "INVALID SELECTION";
    }
    
}

void InputNumbers() {

    cout << "What is the required size of the array? \n";
    cin >> seriesSize;
    cout << "Enter " << seriesSize << " numbers: \n";
    for (int i = 0; i < seriesSize; i++) {
        cin >> numberSeries[i];
    }
    cout << "\n";
}

void PrintArray() {
    cout << "Here are your current numbers: \n";

    for (int i = 0; i < seriesSize; i++) {
        cout << numberSeries[i] << " ";
    }
    
    cout << "\n";
}

int Iteration(int size, int array[]) {
    for (int i = 0; i < (seriesSize - 1); i++) {
        if (numberSeries[i] > numberSeries[i + 1]) {
            SwapNumbers(i, numberSeries);
            counter = 0;
        }
        else if (numberSeries[i] <= numberSeries[i + 1]) {
            counter++;
        }
    }
    if (counter >= seriesSize) {
        cout << "YOUR NUMBERS HAVE BEEN SORTED\n\n";
    }
    else {
        PrintArray();
        cout << "Iteration complete!\n\n";
    }

    return 0;
}

void SwapNumbers(int arrayLocation, int array[]) {
    int tempSwapOne, tempSwapTwo;
    //store each number
    tempSwapOne = array[arrayLocation];
    tempSwapTwo = array[arrayLocation + 1];

    //assign number to new location
    array[arrayLocation] = tempSwapTwo;
    array[arrayLocation + 1] = tempSwapOne;
}

void CreateArrayFromFile(string fileLocation) {
    ifstream file;
    file.open(fileLocation, ios::in);

    int* ptr = (int*)malloc(sizeof(file));

    cout << *ptr;

    string line;
    int i = 0;

    if (file.is_open()) {
        while (!file.eof())
        {
            getline(file, line);
            arr[i] = line;
            i++;
        }
        seriesSize = i;
    }
    else {
        cout << "File could not be opened. Check path is correct...\n\n";
        return;
    }
    
    for (i = 0; i < seriesSize; i++) {
        int tempNumber = stoi(arr[i]);
        numberSeries[i] = tempNumber;
        cout << numberSeries[i] << " ";
    }
    cout << "\nTotal numbers: " << seriesSize;
}

I tried to assign the correct amount of memory but I have no idea how to figure out the correct amount.

You get rid of all the arrays, `malloc`, etc. and use `std::vector` with its `push_back` member function to add new elements instead. Note that `malloc` in particular is very easily wrong in C++. It should be `new` instead at the very least. — user17732522, Dec 20 '22 at 01:49
Protip - use containers like `std::vector` and `std::array` instead of naked arrays. — Captain Obvlious, Dec 20 '22 at 01:49
Aside from that there are a few other problems with your code. I assume you are not yet using a [good introductory book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) to the language, which I would recommend you to do. — user17732522, Dec 20 '22 at 01:54
Are you trying to read the entire contents of a file? For textual data, `std::string` is probably the place to put it. — tadman, Dec 20 '22 at 02:16
Note: `sizeof(file)` is pretty much meaningless garbage. It's an implementation specific quirk, it has nothing to do with the actual size of the file on disk. You're measuring the size of the `std::ifstream` internal representation. To find out the *file's size* there's a number of ways, but calling `seekg()` to the end of the file, and using `tellg()` to find out the offset isn't bad. There's [lots of tools](https://en.cppreference.com/w/cpp/io/basic_istream) available on a stream. — tadman, Dec 20 '22 at 02:16
Don't forget you can skip all of that if you just want a string and [do it super easy](https://stackoverflow.com/questions/2602013/read-whole-ascii-file-into-c-stdstring). — tadman, Dec 20 '22 at 02:19
Recommended reading: [Why is iostream::eof inside a loop condition (i.e. `while (!stream.eof())`) considered wrong?](https://stackoverflow.com/q/5605125/4581301) — user4581301, Dec 20 '22 at 04:40
`numberSeries` is an array with exactly one element. Your function `inputNumbers()` first reads a number of values, then reads that number of values into `numberSeries`. Doing that does not magically resize `numberSeries`, so tromps over memory past the end of the first/only element of `numberSeries`. `numberSeries` needs to be a resizeable collection (e.g. a `std::vector`) - bear in mind that resizing a collection isn't magical - there are some rules you need to follow (e.g. you can't blindly modify elements, and expect a `vector` to always resize itself to suit you). — Peter, Dec 20 '22 at 05:48
*"any tips to improve the overall quality of the code would be greatly appreciated."* -- this is the wrong site for such an open-ended request. For code reviews, try [Code Review](https://codereview.stackexchange.com/tour) (after you get your code working). — JaMiT, Dec 20 '22 at 07:11
*"susceptible to buffer overflow"* -- this is a good start to a question, in that it identifies the problem. However, more details and less code would help. Which buffer? Where in the code does the overflow occur? How did you identify the problem as buffer overflow (what symptoms did you observe)? How much code do you really need to demonstrate the overflow? An overflow could probably be demonstrated in a dozen lines of code, likely half that. — JaMiT, Dec 20 '22 at 07:14
Before you post at [codereview.se], make sure to read [A guide to Code Review for Stack Overflow users](//codereview.meta.stackexchange.com/a/5778), as some things are done differently over there - e.g. question titles should simply say what the code *does*, as the question is always, "How can I improve this?". Be sure that the code works correctly; include your unit tests if possible. You'll likely get some suggestions on making it more efficient, easier to read, and better tested. — Toby Speight, Dec 20 '22 at 11:11

score 3 · Answer 1 · answered Dec 20 '22 at 01:49

3

Use std::vector instead. It handles the memory management and will automatically resize.

answered Dec 20 '22 at 01:49

Unmitigated

76,500
11
62
80

How can I allocate the right amount of memory to this?

1 Answers1