views:

83

answers:

1

I was trying following code in which I defined copy c'tor explicitly to solve aliasing problem.
But code is giving runtime error.

#include<iostream>
#include<cstring>
using namespace std;

class word
{
  public:
    word(const char *s) // No default c'tor
    {
      str=const_cast<char*>(s); 
      cnt=strlen(s);
    }
    word(const word &w)
    {
      char *temp=new char[strlen(w.str)+1];
      strcpy(temp,w.str);
      str=temp;
      cnt=strlen(str);
    }
   ~word()
   {
     delete []str; 
     cout<<"destructor called"<<endl;
   } 
   friend ostream& operator<<(ostream &os,const word &w);

 private:
   int cnt;
   char *str;
};

ostream& operator<<(ostream &os,const word &w)
{
  os<<w.str<<" "<<w.cnt;
  return os;
}

word noun("happy");
void foo()
{
  word verb=noun;
  cout<<"inside foo()"<<endl;
  cout<<"noun : "<<noun<<endl<<"verb : "<<verb<<endl;
}

int main()
{
  cout<<"before foo()"<<endl<<"noun : "<<noun<<endl;
  foo();
  cout<<"after foo()"<<endl<<"noun : "<<noun<<endl;
  return 0;
}
+8  A: 

Problem is in this constructor:

 word(const char *s) // No default c'tor
    {
      str=const_cast<char*>(s); 
      cnt=strlen(s);
    }

Here you are not allocating any memory to copy the string into str variable. But in destructor of the class you are doing delete[] str;, since the memory for the str is not allocated using new[] it is crashing. You need to allocate memory similar to one you are doing in copy constructor and copy the string into the newly allocated memory. Or better still, use std::string.

EDIT: If you really don't want to use std::string for some reason, you also need an assignment operator with check for self assignment as mentioned by @icabod.

Naveen
Also worth pointing out that there's no assignment operator, so in method "foo()" you would actually end up trying to delete the data pointed to by "noun" anyway. If playing with pointers in a class, you really need an "operator=" too.
icabod
@icabod.. Why do I need operator= because in foo(), I am creating a new object, not assigning to any previously constructed.
Happy Mittal
@Happy Mittal: You are not using it in this code, but that doesn't mean you won't use it tomorrow. When you are writing a class and you are writing a copy constructor to it, you *should* write a assignment operator also.
Naveen
@Naveen: Rather than doing a test for self-assignment, use the [copy-and-swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). That's much better in several ways.
sbi