0

I'm trying to chain some functions but right after calling the first function, the destructor is called; then at the end of the scope the destructor is called again.

int i=0;
class Winbitmap{
    public:
    int index=i++;
    Gdiplus::GdiplusStartupInput gdiplusStartupInput;
    ULONG_PTR gdiplusToken;
    Gdiplus::Bitmap* bitmap;
    Winbitmap();
    ~Winbitmap();
    Winbitmap&  getCapture(int,int,int,int);
};
Winbitmap::Winbitmap(){ Gdiplus::GdiplusStartup(&gdiplusToken, &gdiplusStartupInput, NULL); }
Winbitmap::~Winbitmap(){
    //{delete bitmap;}
    std::wcout << L"destructed:" << index << std::endl;
    Gdiplus::GdiplusShutdown(gdiplusToken);
}
Winbitmap& Winbitmap::getCapture(int x=0, int y=0, int w=0, int h=0) { 
    bitmap = new Gdiplus::Bitmap(captureWindow(GetDC(GetDesktopWindow()),x,y,w,h),NULL); 
    std::wcout << L"captured:" << index << std::endl;
    return *this;
}

This is how I intend to use it:

Winbitmap bitmap1 = Winbitmap().getCapture();
std::wcout<<L"confirmed1\n"; 
Winbitmap bitmap2 = Winbitmap().getCapture();
std::wcout << L"confirmed2\n";
//If I try to use any of the bitmaps later, the program hangs

Output:

captured:0
destructed:0
confirmed1
captured:1
destructed:1
confirmed2
destructed:1
destructed:0

How can I return correctly a reference to the object without calling the destructor?

shuji
  • 7,369
  • 7
  • 34
  • 49
  • 1
    The default copy constructor is being used to copy the temporary returned by `getCapture()` into ‘bitmap1`. You forgot to follow the [rule of three](https://en.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)). – Raymond Chen Jul 10 '15 at 03:26
  • I don't see the reason for `bitmap` being a pointer at all. Calling `getCapture` twice in succession will leak memory. – chris Jul 10 '15 at 03:36
  • bitmap is noncopyable so I have to store it in a pointer. – shuji Jul 10 '15 at 03:39
  • 1
    Ah, use a smart pointer then. And if it's called twice in succession, you immediately overwrite the variable with a pointer to the new bitmap memory and the old memory no longer has any pointers to it. – chris Jul 10 '15 at 03:40
  • You're right thank you that's a nice suggestion, but wouldn't that make noncopyable my entire class object? – shuji Jul 10 '15 at 03:41
  • 1
    @shuji, If you use a `std::unique_ptr` and have the default copy operations disabled, you can still implement them manually however you need to. The closer thing to what you're doing now is a `std::shared_ptr`, where each copy of your class would point to the same bitmap and then the bitmap would be cleaned up when all copies of that class object are gone. – chris Jul 10 '15 at 03:50
  • Is `delete bitmap` the proper way to free a `Gdiplus::Bitmap`? Because, if not, you'll need a custom deleter to be able to use `std::unique_ptr`. – celticminstrel Jul 10 '15 at 04:08

2 Answers2

1

The line:

Winbitmap bitmap1 = Winbitmap().getCapture();

Creates a temporary object, calls getCapture() on the temporary object, calls the copy constructor to construct bitmap1, and then destroys the temporary object.

You can use:

Winbitmap const& bitmap1 = Winbitmap().getCapture();

However,

I suggest using:

Winbitmap bitmap1;
bitmap1.getCapture();

That is much clearer to read and understand.

R Sahu
  • 204,454
  • 14
  • 159
  • 270
0

Defining a move constructor should fix your problem, provided you can use C++11 (I'm pretty sure VS2013 does support this):

WinBitmap::WinBitmap(WinBitmap&& bmp) : index(bmp.index), gdiplusToken(bmp.gdiplusToken), bitmap(bmp.bitmap) {
    bmp.bitmap = NULL; // Because the new WinBitmap has now acquired ownership of the pointer.
}

Then your destructor would make sure bitmap is not null before deleteing it.

celticminstrel
  • 1,637
  • 13
  • 21
  • Deleting null is fine. – chris Jul 10 '15 at 04:27
  • You're wrong. Deleting null general raises a segfault. (And even if it doesn't, you don't want to delete null - that's probably the location at which the start of your program's code is.) – celticminstrel Jul 10 '15 at 04:28
  • 1
    See [expr.delete]: *In the first alternative (delete object), the value of the operand of delete may be a null pointer value, a pointer to a non-array object created by a previous new-expression, or a pointer to a subobject (1.8) representing a base class of such an object (Clause 10). If not, the behavior is undefined.* – chris Jul 10 '15 at 04:30
  • Huh, okay, I was unaware of that. But what's the specified behaviour if it _is_ a null pointer? – celticminstrel Jul 10 '15 at 04:31
  • Which standard is that quote from? (C++98, C++11, ...) – celticminstrel Jul 10 '15 at 04:38
  • C++14 (N4140), but I guess I could have said the null pointer behaviour is further down. It doesn't actually say "nothing happens", but the wording was to that effect in C++03 at least. There's a better discussion of the subtle language standard differences [here](http://stackoverflow.com/questions/25329576/calling-delete-on-null-pointers-c03-vs-c11) – chris Jul 10 '15 at 04:44