dacap / clip

Cross-platform C++ library to copy/paste clipboard content
MIT License
622 stars 89 forks source link

Call SHCreateMemStream by ordinal for Windows XP support #67

Closed AaronWright3 closed 1 year ago

AaronWright3 commented 1 year ago

I agree that my contributions are licensed under the MIT License. You can find a copy of this license at https://opensource.org/licenses/MIT

Prior to Vista, SHCreateMemStream was available in Windows XP, but needed to be called by ordinal 12, rather than by name. Doing this doesn't affect functionality in higher versions of windows. Tested with MinGW on 32-bit Windows XP and Windows 11 x64.

AaronWright3 commented 1 year ago

Thanks for being patient my my first pull request and my rite-of-passage wrestling with git commits.

I moved SHCreateMemStream into read_png and put it behind #ifdef CLIP_SUPPORT_WINXP and also added a corresponding CMake option to CMakeLists.txt. I put it in the second if(WIN32) block, just because it seemed wise to group windows-related things together, but I suppose it doesn't have to go anywhere in particular since it will only affect anything if clip_win.cpp is included anyway.

I initially put it outside any function because I wanted to ensure it was only loaded and defined once, just to avoid any redundancy, and I wasn't sure of any better way to do that for a library like this. I would prefer a runtime check for the existence of SHCreateMemStream which then gets loaded by ordinal if it's missing, but I don't think there's any elegant way to do that. Allegedly Windows keeps DLLs loaded based on reference count, so LoadLibrary might not be doing any unnecessary reloading after all if that's true. Plus, the niche use case and the fact that it's just decoding clipboard data means I probably shouldn't worry too much about performance.

Let me know what you think.

dacap commented 1 year ago

@AaronWright3 it looks great as it is, as at the moment we don't have an init/exit for the clip library, so loading/unloading shlwapi.dll in read_png makes sense.

I'll merge this squashed and then do some minor cosmetic/formatting changes.

dacap commented 1 year ago

Some minor changes in 94693e2414a2c69a8ca16f065240c80a94cc6221