1

I am defining a new C++ class whose what method returns a char* type with the value of an integer passed as constructor.

Originally I did it using string class and returning the string data from what.

Then I am trying to use char* type in the following code:

/* Define the exception here */
class BadLengthException: public exception
{
  public:
    BadLengthException(int strLength)
    {
        strLen = strLength;
        res = (char*)malloc(strLength+1);
        int resultSize = sprintf(res, "%d", strLen);
    }
    ~BadLengthException() throw()
    {
        free(res);
    }
    virtual const char* what() const throw()
    {
      return res;
    }
  private:
    int strLen;
    char* res;
};

but I am having problem when freeing the malloc allocated variable: it gives this Exception:

pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

So why is that? Where and how should I free a dyanmic allocated variable in a Exception class?

EDIT

Here a minimal working complete example. The program will ask for user input. The first is a number specifying the number of the following inputs. The other inputs will be strings. The above exception will be raised if the string is shorter than 5.

Just enter: 1 and then Me for example

#include <iostream>
#include <string>
#include <sstream>
#include <exception>
using namespace std;

/* Define the exception here */
class BadLengthException: public exception
{
  public:
    BadLengthException(int strLength)
    {
        strLen = strLength;
        res = (char*)malloc(strLength+1);
        int resultSize = sprintf(res, "%d", strLen);
    }
    ~BadLengthException() throw()
    {
        free(res);
    }
    virtual const char* what() const throw()
    {
      return res;
    }
  private:
    int strLen;
    char* res;
};



bool checkUsername(string username) {
    bool isValid = true;
    int n = username.length();
    if(n < 5) {
        throw BadLengthException(n);
    }
    for(int i = 0; i < n-1; i++) {
        if(username[i] == 'w' && username[i+1] == 'w') {
            isValid = false;
        }
    }
    return isValid;
}

int main() {
    int T; cin >> T;
    while(T--) {
        string username;
        cin >> username;
        try {
            bool isValid = checkUsername(username);
            if(isValid) {
                cout << "Valid" << '\n';
            } else {
                cout << "Invalid" << '\n';
            }
        } catch (BadLengthException e) {
            cout << "Too short: " << e.what() << '\n';
        }
    }
    return 0;
}

EDIT 2

The original class using string is the following: this one does work

class BadLengthException: public exception
{
  public:
    BadLengthException(int strLength)
    {
        res = to_string(strLength);
    }
    virtual const char* what() const throw()
    {
      return res.c_str();
    }
  private:
    string res;
};
roschach
  • 8,390
  • 14
  • 74
  • 124

3 Answers3

6

This has nothing to do with exceptions. Your class is not safe to copy.

If you are going to write a class like that then you need to make it follow the rule of three.

What happening is that your exception object is being copied, which copies the pointer, so you are freeing the same pointer twice.

But the easy way to do this is to use a std::string instead of allocating your own memory.

class BadLengthException: public exception
{
public:
    BadLengthException(int strLength) : strLen(strLength), res(std::to_string(strLength))
    {
    }
    virtual const char* what() const throw()
    {
      return res.c_str();
    }
  private:
    int strLen;
    std::string res;
};
roschach
  • 8,390
  • 14
  • 74
  • 124
john
  • 85,011
  • 4
  • 57
  • 81
  • Yes thanks for the Rule of 3. As already specified in my question I already did with string and it worked – roschach Feb 05 '19 at 14:55
  • But why my exception object is being copied? – roschach Feb 05 '19 at 15:08
  • 1
    @FrancescoBoi Because the C++ standard states that when an exception is thrown it is copied. Often an exception object is a temporary or even a stack variable. It's copied to a global area so it is preserved during the processing of the exception. – john Feb 05 '19 at 15:19
1

Exceptions are supposed to not lead to exceptions themeselves; they are supposed to be noexcept. Therefore, dynamic memory alloc/dealloc - either implicit(e.g using std::string...) or explicitly(new/delete, malloc/free...) - is generally discouraged. One better approach would be using a statically char-array:

class BadLengthException:
    public std::exception
{
public:
    BadLengthException(size_t len){
        std::snprintf(res, bufsz, fmt(), len);
    };
    ~BadLengthException()=default;
    virtual const char* what() const {return res;};
private:
    static auto& fmt() {return "bad length [%dz]";};
    static constexpr size_t fmtsz=sizeof(fmt());
    static constexpr size_t intsz=std::numeric_limits<size_t>::digits10;
    static constexpr size_t bufsz=fmtsz+intsz;
    char res[bufsz];
};
Red.Wave
  • 2,790
  • 11
  • 17
-1

I will add another answer trying to put everything together.

The problem

The problem is the following. Consider this example:

double division(int a, int b) {
   if( b == 0 ) {
      throw "Division by zero condition!";
   }
   return (a/b);
}

In another block the function will be called:

double c;
try
{
    c =division(d,f)
}
catch( ExceptionName e ) {
    ...
}

Normally exceptions are thrown (i.e. the instance is generated) in called functions (division in the example above) but they are catched other parts of the code (direct calling function or even in more outer functions). To make the exception instance available from where it is generated to where the it is catched, the instance is copied.

Copy of exception instances is apparently performed using the copy assignment constructor (= to be clear). Since I did not declare one, the default one is used. This default constructor has the following behaviour with pointers: instead of copying the pointed value, it copies the pointer value itself (i.e. the address) so that now I have another instance pointing to the same address.

Suppose an exception occurs in the division function above. Suppose a copy of the instance is performed. When returning from the function the original instance is being destroyed because it is in the stack memory so the destructor is called and the pointer will be freed. However memory of the pointer is also shared by the new copy.

When the copy will try to use its pointer an error will arise because it is not a valid pointer. In my case it was being used by its destructor that was trying to free it but it was already freed: hence the error.

Rule of 5

The problem arised because of me violating the rule of 5 in C++11 (previously rule of 3).

The rule states that when a resource is used and managed manually (in my case the memory was the resource) and if a class implements one of the following members, also the other should be overridden:

  1. destructor
  2. copy constructor
  3. copy assignment operator
  4. move copy constructor
  5. move assignment operator

4 and 5 are the correspondant of 2 and 3 but with rvalues (here a good post on lvalues rvalues)

Possible solutions

The easiest way is to use string, as already specified in the question, comments and answers so that the memory is automatically managed by the string class.

Another alternative (as posted in another answer) is to use shared pointer.

Although inconvenient, to be coherent with the question, here an implementation using pure pointers. Constructors and destructor are redefined so that new memory is allocated for them in the copy constructors, instead using the one allocated by another instance.

/* Define the exception here */
class BadLengthException: public exception
{
  public:
    BadLengthException(int strLength)
    {
        cout<<"Constructor\n";
        strLen = strLength;
        res = (char*)malloc(strLen+1);
        int resultSize = sprintf(res, "%d", strLen);
    }


    /*assignment operator*/
    BadLengthException& operator= (const BadLengthException &other)
    {
        cout<<"copy assignment constructor"<<endl;
        if(&other == this)
        {
            return *this;
        }
        strLen = other.strLen;
        res = (char*)malloc(strLen+1);
        int resultSize = sprintf(res, "%d", strLen);
        return *this;
    }

    /*copy constructor*/
    BadLengthException(BadLengthException& other)
    {
        cout<<"copy constructor\n";
        *this = other;

    }

    BadLengthException(BadLengthException&& other)
    {
        cout<<"move constructor "<<endl;
        *this = other;
    }


    BadLengthException& operator=(BadLengthException&& other)
    {
      cout<<"move assignment operator"<<endl;
      if(&other == this)
      {
          return *this;
      }
      *this = other;
      return *this;

    }

    /*class destructor*/
    ~BadLengthException() throw()
    {
        cout<<"destructor"<<endl;
        free(res);
    }

    virtual const char* what() const throw()
    {
      return res;
    }
  private:
    int strLen;
    char* res;
};

Example to test the class:

int main(int argc, char *argv[])
{
  {
    cout<<"-----Expecting copy constructor------\n";
    BadLengthException e(10);
    BadLengthException e1(e);
    cout<<"10: "<<e1.what()<<endl;
  }
  cout<<endl<<endl;
  {
    cout<<"-----Expecting copy assignment operator------\n";
    BadLengthException e(10);
    BadLengthException e2 = e;
    cout<<"10: "<<e2.what()<<endl;
  }
  cout<<endl<<endl;
  {
    cout<<"-----move assignment operator------\n";
    BadLengthException e3(1);
    e3 = BadLengthException(33);
  }
  {
    cout<<"-----move constructor------\n";
    BadLengthException e4 = BadLengthException(33);
  }
  {
    cout<<"-----move constructor------\n";
    BadLengthException e5(BadLengthException(33));
  }
  cout<<"-----6------\n";
  BadLengthException e6(1), e6_1(2);
  e6 = std::move(e6_1);
  return 0;
}
roschach
  • 8,390
  • 14
  • 74
  • 124