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!

just use std::vector since you're using a c++ compiler anyway — Ryan
– Ryan, Commented May 4, 2015 at 1:59
You're trying to do several new things at once; tackle them one at a time. — Beta
– Beta, Commented May 4, 2015 at 2:00
I would, but I need to learn how to use malloc(). And I'm having some trouble trying to figure it out. — WaterlessStraw
– WaterlessStraw, Commented May 4, 2015 at 2:00
It looks like you are using malloc() fine. Can you explain what you are doing here: if (count == 0){ //if count is 0, don't add to array count++; std::cout << "count++" << std::endl; } — bentank
– bentank, Commented May 4, 2015 at 2:09

bentank · Accepted Answer · 2015-05-04 02:55:13Z

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

edited May 4, 2015 at 2:55

answered May 4, 2015 at 2:35

bentank

6163 silver badges7 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

WaterlessStraw Over a year ago

@bentank Thanks for the feedback! It looks like everything is working correctly, except the last value is added to the array twice. I'll have to look into it. Thank you!

Community · Accepted Answer · 2017-05-23 12:14:16Z

0

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:

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.
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?

edited May 23, 2017 at 12:14

CommunityBot

11 silver badge

answered May 4, 2015 at 2:56

Ben Voigt

286k45 gold badges444 silver badges764 bronze badges

1 Comment

WaterlessStraw Over a year ago

Great, thank you for pointing this out. This is something my professor would not have brought up with me so I'm thankful you took the time to write a response. I edited my loop to while ((count < size) && (fin >> n)) and it works perfectly.

Collectives™ on Stack Overflow

Reading number list from file to a dynamic array

2 Answers 2

1 Comment

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related