Reading number list from file to a dynamic array

Question

I'm having trouble reading a number list from a .txt file to a dynamic array of type double. This first number in the list is the number of numbers to add to the array. After the first number, the numbers in the list all have decimals.

My header file:

#include <iostream>

#ifndef SORT
#define SORT

class Sort{
private:
    double i;
    double* darray; // da array
    double j;
    double size;

public:
    Sort();
    ~Sort();

    std::string getFileName(int, char**);
    bool checkFileName(std::string);
    void letsDoIt(std::string);
    void getArray(std::string);

};

#endif

main.cpp:

#include <stdio.h>

#include <stdlib.h>
#include "main.h"

int main(int argc, char** argv)
{
Sort sort;
    std::string cheese = sort.getFileName(argc, argv); //cheese is the file name

    bool ean = sort.checkFileName(cheese); //pass in file name fo' da check

    sort.letsDoIt(cheese); //starts the whole thing up

   return 0;
 }

impl.cpp:

#include <iostream>
#include <fstream>
#include <cstring>
#include <stdlib.h>

#include "main.h"

Sort::Sort(){
    darray[0];
    i = 0;
    j = 0;
    size = 0;


}
Sort::~Sort(){
    std::cout << "Destroyed" << std::endl;
}
std::string Sort::getFileName(int argc, char* argv[]){
    std::string fileIn =  "";
    for(int i = 1; i < argc;)//argc the number of arguements
    {
        fileIn += argv[i];//argv the array of arguements
        if(++i != argc)
            fileIn += " ";
    }
    return fileIn;
}
bool Sort::checkFileName(std::string userFile){
    if(userFile.empty()){
        std::cout<<"No user input"<<std::endl;
        return false;
    }
    else{

        std::ifstream tryread(userFile.c_str());
        if (tryread.is_open()){
            tryread.close();
            return true;
        }
        else{
            return false;
        }
    }

}
void Sort::letsDoIt(std::string file){
    getArray(file);

}
void Sort::getArray(std::string file){

    double n = 0;
    int count = 0;
    // create a file-reading object
    std::ifstream fin;
    fin.open(file.c_str()); // open a file
    fin >> n; //first line of the file is the number of numbers to collect to the array
    size = n;
    std::cout << "size: " << size << std::endl;

    darray = (double*)malloc(n * sizeof(double));  //allocate storage for the array

  // read each line of the file
    while (!fin.eof())
    {
        fin >> n;
        if (count == 0){ //if count is 0, don't add to array
            count++; 
            std::cout << "count++" << std::endl;
        }
        else {
            darray[count - 1] = n; //array = line from file
            count++;
        }


    std::cout << std::endl;
  }
     free((void*) darray); 
}

I have to use malloc, but I think I may be using it incorrectly. I've read other posts but I am still having trouble understanding what is going on.

Thanks for the help!

Answer 1

Your use of malloc() is fine. Your reading is not doing what you want it to do.

Say I have the inputfile:

3
1.2
2.3
3.7

My array would be:

[0]: 2.3
[1]: 3.7
[2]: 0

This is because you are reading in the value 1.2 as if you were rereading the number of values.

When you have this line:

fin >> n; //first line of the file is the number of numbers to collect to the array

You are reading in the count, in this case 3, and advancing where in the file you will read from next. You are then attempting to reread that value but are getting the first entry instead.

I believe that replacing your while() {...} with the code below will do what you are looking for.

while (count != size && fin >> n)
{
    darray[count++] = n; //array = line from file
    std::cout << n << std::endl;
}

This should give you the correct values in the array:

[0]: 1.2
[1]: 2.3
[2]: 3.7

Answer 2

You appear to be writing the next exploitable program. You are mistakenly trusting the first line of the file to determine your buffer size, then reading an unlimited amount of data from the remainder of the file into a buffer that is not unlimited. This allows an evil input file to trash some other memory in your program, possibly allowing the creator of that file to take control of your computer. Oh noes!

Here's what you need to do to fix it:

1. Remember how much memory you allocated (you'll need it in step #2). Have a variable alleged_size or array_length that is separate from the one you use to read the rest of the data.

2. Don't allow count to run past the end of the array. Your loop should look more like this:

   while ((count < alleged_size) && (cin >> n))
   

This both prevents array overrun and decides whether to process data based on whether it was parsed successfully, not whether you reached the end-of-file at some useless point in the past.

The less problematic bug is the one @bentank noticed, that you didn't realize that you kept your position in the file, which is after the first line, and shouldn't expect to hit that line within the loop.

In addition to this, you probably want to deallocate the memory in your destructor. Right now you throw the data away immediately after parsing it. Wouldn't other functions like to party on that data too?