#280 atldlgs.h Missing GlobalUnlock Calls

[Anonymous]

atldlgs.h contains numerous instances of GlobalLock being called, without ever calling a corresponding GlobalUnlock

e.g. CPageSetupDialogImpl's GetDevMode, GetDriverName, GetPortName - pretty much any GetSomething call in any class in the file.

I don't really know whether this has been done on purpose or whether this is something experienced programmers should "expect". My perspective is if the caller does not look at the implementation details they may not know they need to call GlobalUnlock once they are finished.

I would imagine this could be resolved by adding a bunch of code to the destructors of all affected classes that tries to GlobalUnlock...maybe if t_bManaged == true or something? I don't know whether this could cause issues from a "design" perspective for WTL

[Nenad Stefanovic]

Described here are the classes CPrintDialogImpl/CPrintDialogExImpl/CPageSetupDialogImpl that all have methods GetDevMode()/GetDriverName()/GetDeviceName()/GetPortName().

The implementation of these methods is based on MFC and they do it the same way. MFC documentation for GetDevMode() states that the caller should call GlobalUnlock(). It is the same for other methods, although you can't use the returned value and need to unlock m_pd.hDevNames.

However, GlobalLock() and GlobalUnlock() don't really do anything in Win32. They were important in 16-bit Windows and are here today only for compatibility reasons. GlobalFree() will free memory that is locked and there will be no memory leaks.

No change needed here.

[Anonymous]

Yeah, that seems to be the case. Further research reveals the real issue is that PrintDlg/PrintDlgEx allocates memory for your PRINTDLG/PRINTDLGEX structure's hDevMode and hDevNames attributes if these values are NULL (which they are). This appears to be a known "thing" programmers have to deal with when working with print dialogs, however this could be remediated by adding a GlobalFree in the destructors of CPrintDialogImpl and CPrintDialogExImpl (as well as CPageSetupDialogImpl, I would assume)

Would this be something WTL would be interested in? I can see that CDevModeT in atlprint.h actually does this already (in its Cleanup method), so this form could easily be copied and modified for hDevMode and hDevNames in the destructors of CPrintDialogImpl/CPrintDialogExImpl/CPageSetupDialogImpl

[Nenad Stefanovic]

It would be possible to do that, but it's not that simple. After 15+ years of doing it this way, a change like that couldc ause some problems. Nothing serious, but maybe not necesssary.

We can revisit this for the next version of WTL, especially if there are more people supporting the change. This could be a good topic for a post on Discussion.