Invalid allocation size? c++

Question

I got three files:

Text.h

#include <string>

namespace w3
{
    class Text
    {
        std::string file_name;
        std::string* handler;
        int records;

    public:
        Text();
        Text(char*);
        size_t size() const;
        Text(const Text& );
        Text& operator=(const Text&);
        Text(Text&&);
        Text&& operator=(Text &&);
        ~Text();
    };
}

Text.cpp

#include <iostream>
#include <string>
#include <fstream>
#include <cstring>
#include "Text.h"

namespace w3
{
    Text::Text()
    {
        file_name = "";
        handler = nullptr;
        records = 0;
    }

    Text::Text(char* file)
    {
        file_name = file;

        if (file[0]='\0')
        {
            file_name=" ";
            std::cout << "can not find file name !!" <<std::endl;
        }
        else
        {
            std::fstream f(file_name);
            std::string line;

            if (f.is_open()) //ERROR
            {
                while (std::getline(f,line,'\n'))
                {
                    records++;
                }
            }
            else
            {
                std::cout << "file not open !!" <<std::endl;
            }

            std::string* handle = new std::string[size()];

            f.clear();
            f.seekg(0,std::ios::beg);

            int counter = 0;

            while (std::getline(f,line,'\n'))
            {
                if (counter != records)
                {
                    handle[counter]=line;
                    counter++;
                }
            }
        }
    }

    size_t Text::size() const
    {
        return (size_t)records;
    }

    Text::Text(const Text& src)
    {
        std::string file_name = src.file_name;

        int no_of_rec = src.records;

        if (src.handler != nullptr)
        {
            handler = new std::string[src.size()];
            handler = src.handler;
        }
        else
        {
            handler = nullptr;
        }
    }

    Text& Text::operator=(const Text& src)
    {
        if (this != &src)
        {
            int no_of_rec = src.records;

            std::string file_name = src.file_name;

            if (src.handler != nullptr)
            {
                handler = new std::string[src.size()];
                handler=src.handler;
            }
            else
            {
                handler = nullptr;
            }
        }
        return *this;
    }

    Text::Text(Text&& src)
    {
        file_name=src.file_name;

        handler = src.handler;

        records = src.records;

        src.file_name = " ";
        src.handler = nullptr;
        src.records = 0;
    }

    Text&& Text::operator=(Text&& src)
    {
        if (&src != this)
        {
            file_name = src.file_name;

            handler = src.handler;

            records = src.records;

            src.file_name = " ";

            src.handler = nullptr;

            src.records = 0;

        }
        return std::move(*this);
    }

    Text::~Text()
    {
        //delete [] handler;
    }
}

w3.cpp

 #include <iostream>
 #include <iomanip>
 #include <utility>
 #include <ctime>
 #include "Text.h"
 #define TIME(start, end) double((end) - (start)) / CLOCKS_PER_SEC

 int main (int argc, char* argv[]) {
     if (argc == 1) {
         std::cerr << argv[0] << ": missing file operand\n";
         return 1;
     }
     else if (argc != 2) {
         std::cerr << argv[0] << ": too many arguments\n";
         return 2;
     }
     std::clock_t cs, ce;
     {
         std::cout << std::fixed << std::setprecision(3);
         cs = std::clock();
         w3::Text a;
         ce = std::clock();
         std::cout << "Constructor      " << TIME(cs, ce) << " seconds"; 
         std::cout << " - a.size = " << a.size() << std::endl;

         cs = std::clock();
         w3::Text b(argv[1]);
         ce = std::clock();
         std::cout << "Constructor      " << TIME(cs, ce) << " seconds";
         std::cout << " - b.size = " << b.size() << std::endl;

         cs = std::clock();
         a = b;
         ce = std::clock();
         std::cout << "Copy Assignment  " << TIME(cs, ce) << " seconds";
         std::cout << " - a.size = " << a.size() << std::endl;

         cs = std::clock();
         a = std::move(b);
         ce = std::clock();
         std::cout << "Move Assignment  " << TIME(cs, ce) << " seconds";
         std::cout << " - a.size = " << a.size() << std::endl;

         cs = std::clock();
         w3::Text c = a;
         ce = std::clock();
         std::cout << "Copy Constructor " << TIME(cs, ce) << " seconds";
         std::cout << " - c.size = " << c.size() << std::endl;

         cs = std::clock();
         w3::Text d = std::move(a);
         ce = std::clock();
         std::cout << "Move Constructor " << TIME(cs, ce) << " seconds";
         std::cout << " - d.size = " << d.size() << std::endl;

         cs = std::clock();
     }
     ce = std::clock();
     std::cout << "Destructor       " << TIME(cs, ce) << " seconds\n";
 }

I get an error at the function Text::Text(char* file) at the line of if (f.is_open()).

Now, I debugged and concluded that the error results in a Invalid allocation size.

When I further inspected the error, I noticed that the records value would have an absurd negative number such as -858993460!

I need pointers on how to fix this.

The text file that is being opened is rather a really large file...

Thanks.

Answer 1

Let's start by saying that you could eliminate major portions of this code, plus have little chance of bugs occurring if you used std::vector<std::string> as the handler member, instead of std::string*.

Having said that, the one glaring error is that you have a faulty copy constructor. You are not copying all of the information from the Text object that is passed to you. You left out the file_name and records members, leaving them uninitialized.

In addition, your handler member variable is set incorrectly -- you allocate memory, and then you set this->handler again to a pointer to the passed-in object's handler member. This is totally incorrect.

The copy constructor should allocate a new array, and then copy the strings from the passed-in object to the new object's handler member.

Your copy constructor should look something like this:

Text::Text(const Text& src) : file_name(src.file_name),
                              records(src.records),
                              handler(nullptr)
{
    if ( src.handler )
    {
        handler = new std::string[src.records];
        for (size_t i = 0; i < src.records; ++i )
            handler[i] = src.handler[i];
    }
}

Once you have this, remove the comment from the destructor -- delete [] handler; should be uncommented.

Once this is all done, the assignment operator is simple:

#include <algorithm>
//...
Text& operator=(const Text& src)
{
   Text tmp(src);
   std::swap(tmp.handler, handler);
   std::swap(tmp.records, records);
   std::swap(tmp.file_name, file_name);
   return *this;
}

This uses the copy / swap idiom. Basically all that this does is create a temporary object from the passed-in object, and swap out its internals with this's internals. Then the temporary object dies with the old data.