Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does CDC::SelectObject(CFont*) accept a CFont object vs. a pointer?

Tags:

c++

mfc

    CPaintDC dc(this); 
    CFont font; 
    dc.SelectObject(font);  // why does this build? 

The function CDC::SelectObject takes a pointer of type CFont but why does this build with providing an object? I came across this issue that the above code is unpredictable and can crash sometimes but not always.

like image 519
zar Avatar asked Feb 16 '18 17:02

zar


1 Answers

The code in question is valid, sort of. It compiles due to a combination of two things:

  • The CFont::operator HFONT().
  • An undocumented CDC::SelectObject overload taking an HGDIOBJ handle.

When the compiler tries to match an overload for dc.SelectObject(font), none of them match. So next it tries the user-defined conversion operator (operator HFONT()) that returns an HFONT. That matches the undocumented overload taking an HGDIOBJ (both HGDIOBJ and HFONT are typedef'd as void*).

The code posted in the question is almost correct, too, with 2 exceptions:

  • The font object is destroyed while still being selected into a device context. While this causes a double-delete bug (the font object is owned by the CFont instance and the device context), the call to DeleteFont gracefully fails when passed an invalid handle.
  • The font object previously selected into the device context is lost, and thereby leaked.

Neither issue causes unpredictable behavior or intermittent crashes. As explained in your answer, the real code, however, looks like this:

CFont* pOldFont = (CFont*) dc.SelectObject(font);

That is a bug that exhibits undefined behavior. dc.SelectObject(font) returns an HGDIOBJ (typedef'd as void*), which is subsequently cast to an unrelated type (CFont*). While storing the font previously selected into the device context to later restore it is correct, the code is not. An implementation that honors ownership could be:

CPaintDC dc(this);
CFont font;
CFont oldFont;
// Transfer ownership of font to the DC, and the previously selected font into oldFont
oldFont.Attach(dc.SelectObject(font.Detach()));

// Use dc

// Transfer ownership back
font.Attach(dc.SelectObject(oldFont.Detach()));

// oldFont goes out of scope; this a no-op since it no longer owns any resources
// font goes out of scope, releasing all resources owned by it
// dc goes out of scope, releasing all resources owned by objects selected into it

If you are willing to temporarily sacrifice strict ownership semantics, you can make your life easier by using the more standard implementation:

CPaintDC dc(this); 
CFont font; 
CFont* pOldFont = dc.SelectObject(&font);

// Use dc

dc.SelectObject(pOldFont);

This is safe, even if you exit early, without restoring the device context. It would still lead to a double-delete on the font object owned by both the CFont instance and the device context (which is handled gracefully by the API). It doesn't exhibit a font leak, though, because things are more complicated than it seems: There is another invisible owner involved here, a map controlled by MFC that stores temporary objects (like those returned from CGdiObject::FromHandle, which SelectObject(CFont*) calls). The temporary objects are cleaned up as part of MFC's idle time processing.

like image 81
IInspectable Avatar answered Nov 15 '22 08:11

IInspectable