Prevent warning C4273 for functions defined in libunshield.h#227
Prevent warning C4273 for functions defined in libunshield.h#227ScaledLizard wants to merge 2 commits into
Conversation
…ns should be defined and implemented with consistent UNSHIELD_API (MSVC DLL builds)
|
My understanding is that this is only needed for the declaration of a function (usually *.h) not for the definition of a function (usually *.c). With my current knowledge these changes are not needed, the CMake files need changes. |
|
Thanks for taking a look 🙂 You're right that on many platforms (especially GCC/Clang on Linux), it is usually sufficient to annotate only the declarations in the headers. However, if the declaration uses UNSHIELD_API but the definition does not, MSVC reports: warning C4273 (inconsistent DLL linkage) Adding UNSHIELD_API to the definitions ensures that declaration and definition match, which resolves these errors. You can probably reproduce this only on MSVC (tested in version 2022). Also, i tested compiling unmodified unshield on MinGW64, and that worked without problems, so #225 seems unrelated. |
|
I have some scars from C/C++ development in Windows back in the 1990s so I see this as perfectly reasonable by Microsoft standards :) |
|
@ScaledLizard I've added a workflow which is using MSVC to #225 - it does build without errors as is - some parts are missing, see last comment in #225 |
|
Admittedly, this is only about warnings, but warnings can be confusing as well. Some warnings actually are errors. But no rush, happy easter. |
Zero warnings is the goal! |
…ressed, unshield_file_flags_raw
|
@ScaledLizard #225 is nearly ready (just need to add openssl + zlib dll) to the artifacts, maybe you can give it a try and see if you can reproduce the issue reported here? |
|
@kratz00 wrote: @ScaledLizard #225 is nearly ready (just need to add openssl + zlib dll) to the artifacts, maybe you can give it a try and see if you can reproduce the issue reported here? Sorry, I don't see the relation between issue #225 and #227. MinGW and MSVC are obviously completely different compilers. I'm not involved in issue #225 and I don't have a test setup for it, but I tried compiling unshield 1.6.2 with MinGW and that worked (file attached). Sorry, I'm not sure how to help with this. |
|
#225 also adds a workflow building unshield using MSVC. It does compile without warnings (with the current set of compiler flags) and does so without the changes of this pull requests. It might be different usages of compiler flags, MSVC or something else, but so far I am still not convinced that the changes of this pull request are really needed 😄 |
|
When building libunshield-1.6.2 (unmodified, UNSHIELD_DYNAMIC_LIBRARY defined, as a DLL), I get warning C4273 for numerous functions, including directory.c (please excuse the German). Doing the same build with the fixes from issue #225, these warnings are not shown. However, AFAIK, the underlying issue of #227 is not solved. I don't know why are warnings are not shown anymore. I would vote to apply my proposed fix for issue #227 in addition to the fixes for issue #225. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Let's have consistent signatures and potentially fix these in twogood#227 Co-authored-by: Steffen Pankratz <kratz00@gmx.de> Signed-off-by: David Eriksson <david@activout.se>
When building libunshield as a Windows DLL with MSVC, several functions are declared with UNSHIELD_API in headers but defined without it in source files. This leads to:
warning C4273 (inconsistent DLL linkage)
This patch ensures that all functions declared with UNSHIELD_API
are consistently annotated in their definitions as well.
Also, adds missing include of "internal.h" in log.c so that UNSHIELD_API is available.
Tested with:
MSVC 2022 (DLL + static builds)
Linux (no change in behavior)
No functional changes for non-Windows platforms.