Dynamic string array

Question

I have problem with the following class. I think the problem is with string array, cause I made two other classes and the problem was the same. When I run the program it throws "double free or corruption", but I do not think any double corruption is possible. The problem is same with input string as reference or as common argument in Add method.

class WareH
{
        public:
        WareH(void)
        {
               first = true;
               rows = 1;
               inLine = 0;
               cnt = 0;
               max = 2;
               cL = 0;
               strs = new string[max];
        }
        ~WareH(void)
        {
               delete [] strs;
        }
        bool    Add(string& str, int ending)
        {
               if (first)
                     inLine++;
               else
                     cL++; 
               if (ending == 0)
               {
                     if (first)
                          first = false;
                     if (cL != inLine)
                          return false;
                     rows++;
               } 
               strs[cnt++] = str;
               Bigger();
               return true;
         }
         void Bigger(void)
         {
                if(max == cnt)
                {
                       max *= 2;
                       string* tmp = new string[max];
                       for (int i = 0; i < cnt; i++)
                              tmp[i] = strs[i];
                       delete [] strs;
                       strs = tmp;
                }
         }
         friend ofstream& operator<<(ofstream& of,WareH war)
         {
                for (int a = 0; a < war.cnt; a++)
                       of << war.strs[a] << endl;
                return of;
         }
private:
      bool first;
      int rows, inLine, cnt, max, cL;
      string* strs;
};

Whenever I hear "dynamic" and "array" in the same sentence, I think about `std::vector`. Maybe you should too? — Some programmer dude, Mar 14 '13 at 11:51
Learn the chant... [Rule Of Three... Rule Of Three... Rule Of Three...](http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29) — Alex Chamberlain, Mar 14 '13 at 11:53
@MikeSeymour It's not a duplicate of the famous *What is the Rule of Three?* question, as the OP doesn't know about the Rule of Three at all. — Alex Chamberlain, Mar 14 '13 at 11:54
Simply use `std::vector` and all your problems will go away. — Michael Wild, Mar 14 '13 at 11:54
@AlexChamberlain: Indeed; so an answer to "What is the Rule of Three?" will also answer this question. — Mike Seymour, Mar 14 '13 at 11:55
@MikeSeymour Yes, but the question itself is not a duplicate. — Alex Chamberlain, Mar 14 '13 at 11:55

score 1 · Answer 1 · edited May 23 '17 at 10:25

When a class manages resources, and releases them in its destructor, you must consider the Rule of Three to make sure that copying an object will not result in two objects managing the same resource.

That is what is happening here: the default copy constructor and copy-assignment operator will copy the pointer, giving you two objects which will both try to delete the same array on destruction. Solutions are:

Delete the copy constructor and copy-assignment operator to prevent copying; or
Implement them to copy the strings into a new array, not just the pointer; or
Use std::vector rather than messing around managing memory allocation yourself.

+1. I would shorten it though. "Use `std::vector` unless you have requirement that `std::vector` can't satisfy." I don't see one here. — Jan Hudec, Mar 14 '13 at 12:04

score 0 · Answer 2 · edited May 23 '17 at 11:50

When I run the program it throws "double free or corruption", but I do not think any double corruption is possible.

Educated guess here:

The problem is not in the code you've shown, but in the client code. Here's what I think happens:

you wrote client code that instantiates (or assigns or returns by value or stores in a std container) WareH instances, and since you do not define a copy constructor and assignment operator (see "The Big Three"), they end up copying the values from your source objects. When the first of these instances (that are assigned to each other) are deleted, they delete the strs pointer.

When the second instance is deleted, they delete the same strs pointers that were deleted before (because the default copy constructors and assignment operators do not duplicate the allocated memory but just copy the pointers).

Solutions (if that is indeed, the problem):

working (and bad) solution: explicitly define copy construction and assignment operator for your class.
working (and good) solution: implement your strs as a std::vector<std::string> instead of std::string* and cnt.

But I work only with one instance of the class, even I did test and found out the destructor was called only once. — user1890078, Mar 14 '13 at 13:09

Dynamic string array

2 Answers2