c++ how to solve my memory leak?

Question

Consider the following implemntation of a Binary class representation of an integer:

class Binary {
private:
    int *digits;
    int first1;
public:
    Binary() {
        digits = new int[128];
        digits[0]=0;
        first1=0;
    }
    ~Binary() {
        cout<<"deleting"<<endl;
        delete [] digits;
    }
    Binary(const Binary& b){
        digits = new int[128];
        memcpy(digits,b.digits,128*sizeof(int));
        first1=b.first1;
    }
    explicit Binary(double x){
        int n=(int)x,i;
        digits = new int[128];
        for (i=0; n ; i++,n>>=1) digits[i]=(n & 1)? 1:0;
        first1=i-1;
    }
    Binary& operator =(const Binary& b){
        if (this==&b) return *this;
        memcpy(digits,b.digits,128*sizeof(int));
        first1=b.first1;
        return *this;
    }
    Binary(int n) {
        int i;
        digits = new int[128];
        for (i=0; n ; i++,n>>=1) digits[i]=(n & 1)? 1:0;
        first1=i-1;
    }
    void print() {
        for (int i=first1; i>=0 ; i--) cout<<digits[i];
        cout<<endl;
    }
    operator int() {
        int x = 1,sum=0;
        for (int i=0; i<=first1 ; i++,x<<=1) sum+=digits[i]*x;
        return sum;
    }
    Binary& operator +(Binary& a) {
        int overflow = 0;
        Binary* b1=new Binary();
        int max = a.first1>this->first1? a.first1 : this->first1,bit;
        for (int i=0; i<=max ; i++) {
            bit=a.digits[i]+this->digits[i]+overflow;
            overflow=0;
            if (bit>1) overflow=1;
            b1->digits[i]=bit%2;
        }
        return *b1;
    }

};

and the main using it:

int main() {
    Binary b(91);
    int x=9;
    Binary *b2=new Binary();
    *b2=b+x;
    x=*b2;
    b2->print();
    cout<<" = "<<x;
    cin>>x;
}

lets talk about the line:

*b2=b+x;

first the compiler implicitly allocates a new binary instance for int x, then using it as a paramater for the addition, then creates a new binary instance for the addition result and copies it bit by bit to *b2.

The problem is, that if you run this code, it only prints deleting ONCE, while there were 2 objects created for the execution of the command. apparently there's a leak comes from the addition code, in which i explicitly created a new object to return the result.

Q1: am i correct?

Q2: what can i do to overcome this?

EDIT: The answer and more about the topic of operator overloading can be found here

Answer 1

Summary: Objects allocated with new must be deleted with delete. Objects allocated with new[] must be deleted with delete[]. Globals and locals are deleted automatically when their scope/TU execution ends. In Binary& operator +(Binary& a) you make a Binary that is leaked, and in main you make another Binary that is leaked.

These problems would be avoided if wrote operator+ like so:

Binary operator +(Binary& a) const{ //return by value
    Binary b1(*this); //hold it on the stack
    //math here
    return b1; //return by value
}

and if in main you avoided allocation as well:

Binary b2 = b+x;
x = b2;
b2.print();

This will be faster than your original code, is easier to read and understand, and won't leak.

[Other notes]

Use a std::vector for the internal data instead of managing your own dynamic array. vector is easier, and less likely to make mistakes.

It's usually best to make conversions (like int -> Binary) explicit wherever you can. It adds typing, but saves headaches. This goes for your int conversion operator as well.

Make your const functions const. Right now, if you get a const Binary, you can't do almost anything with it. You can't print it, you can't add anything to it...

You appear to be storing one bit per int, which means you're using about 97% more space than needed (wasting 99.99999995% of the values), which is just silly. Most novices start with 0-9 per char, which only wastes 50% of the space. (though that's still 96% of the values), but is really easy to understand.

The normal way to do addition is like this:

Binary& operator+=(const Binary& rhs) { 
    int max = a.first1>this->first1? a.first1 : this->first1,bit;
    for (int i=0; i<=max ; i++) {
        bit=a.digits[i]+this->digits[i]+overflow;
        overflow=0;
        if (bit>1) overflow=1;
        b1->digits[i]=bit%2;
    }
}  
Binary friend operator+(Binary lhs, const Binary& rhs) {  
{return lhs+=rhs;}

Answer 2

You're not deleting b2 at the end of main.

Also operator should return by value, not returning an allocated Binary object as that will leak because C++ is not garbage collected

Also operator+ should take a const reference.

Also, print, operator+ and operator int should be const member functions

Also, you don't need to dynamically allocate the 128 ints, just make it an array of 128 ints

private:
    int digits[128];

and remove the delete on digits

also, you should initialize it in the constructor with

memset(digits, 0, sizeof(digits));

Answer 3

If you really see "deleting" only once, than this must be the variable b. *b2 = b+x may be done by converting b to int, adding x, constructing a Binary from it and copying that to the place where b2 points to. As b2 is just a raw pointer, you are leaking the initial *b2 instance and the one that overwrites it.