libimobiledevice icon indicating copy to clipboard operation
libimobiledevice copied to clipboard

move LIBIMOBILEDEVICE_API into the headers

Open neheb opened this issue 4 years ago • 9 comments

clang + lld + ucrt on Windows seems to require that the dllimport attribute be applied to the first declaration.

Fixes -Wdll-attribute-on-declaration error.

Signed-off-by: Rosen Penev [email protected]

neheb avatar May 07 '21 09:05 neheb

It would be really great if we can get merged. It's one of the first patches you need to apply to get libimobiledevice to compile natively on Windows.

Although the change itself is trivial, it does require you to update a large amount of files, so it would be awesome if the libimobiledevice project itself could take the patch. It's an (up-to-date) variant of #196.

qmfrederik avatar Jun 04 '21 12:06 qmfrederik

Any update about this @nikias ?

didix21 avatar Aug 17 '21 16:08 didix21

The ugly part of this this solution is that the public headers, and thus also the generated documentation, will show the LIBIMOBILEDEVICE_API prefix in front of ALL public functions. Also there's a #ifdef HAVE_FVISIBILITY check which is a configure time test. This could be worked around with __has_attribute(visibility), but anyway the declspec/attribute part is not externally required so it shouldn't be unnecessarily publicly exposed. We need to find a different solution here. For Windows there's also a way to define exported functions via .def file, maybe that's something to look into.

nikias avatar May 02 '22 09:05 nikias

mind, you might want to compile it as static, and then dllexport/import will cause issues on windows compiler

for visual studio i use _WINDLL as attribute for dynamic (dll) target compilation also for library consumer dllexport doesn't work, you must use dllimport instead.

so basically what you want for windows is static lib compilation nodecoration dynamic dll compilation dllexport other project consuming static lib nodecoration other project consuming dynamic dll dllimport

as example from different project (PODOFO library)

#if defined(_WIN32)
    #if defined(COMPILING_SHARED_PODOFO)
        #define PODOFO_API __declspec(dllexport)
        #define PODOFO_DOC_API __declspec(dllexport)
    #elif defined(USING_SHARED_PODOFO)
        #define PODOFO_API __declspec(dllimport)
        #define PODOFO_DOC_API __declspec(dllimport)
    #else
        #define PODOFO_API
        #define PODOFO_DOC_API
    #endif
    /* PODOFO_LOCAL doesn't mean anything on win32, it's to exclude
     * symbols from the export table with gcc4. */
    #define PODOFO_LOCAL
#else

mexmer avatar May 02 '22 10:05 mexmer

@nikias LIBIMOBILEDEVICE_API can be defined using pkgconfig as dllimport or blank.

The def solution requires removal of LIBIMOBILEDEVICE_API completely.

neheb avatar Apr 25 '23 16:04 neheb

The ugly part of this this solution is that the public headers, and thus also the generated documentation, will show the LIBIMOBILEDEVICE_API prefix in front of ALL public functions. Also there's a #ifdef HAVE_FVISIBILITY check which is a configure time test. This could be worked around with __has_attribute(visibility), but anyway the declspec/attribute part is not externally required so it shouldn't be unnecessarily publicly exposed. We need to find a different solution here. For Windows there's also a way to define exported functions via .def file, maybe that's something to look into.

One method to solve this is to define LIBIMOBILEDEVICE_API to nothing in public headers, unless already defined.

#ifndef LIBIMOBILEDEVICE_API
#define LIBIMOBILEDEVICE_API
#endif //LIBIMOBILEDEVICE_API 

https://github.com/tihmstar/libimobiledevice/blob/master/include/libimobiledevice/libimobiledevice.h#L38-L40

Then in private headers, define it to honor your compile time config

#ifdef WIN32
#define LIBIMOBILEDEVICE_API __declspec( dllexport )
#else
#ifdef HAVE_FVISIBILITY
#define LIBIMOBILEDEVICE_API __attribute__((visibility("default")))
#else
#define LIBIMOBILEDEVICE_API
#endif
#endif

https://github.com/tihmstar/libimobiledevice/blob/master/src/idevice.h#L40-L48

Finally, you need to make sure to include the private header before your public header in all of your c files, so that during compile time your "real" config is honored.

When then libimobiledevice is included in a 3rd party project, LIBIMOBILEDEVICE_API is defined to nothing, as fvisibility information is no longer needed at this point. In case of windows, you may actually check if LIBIMOBILEDEVICE_API is already defined and if not, define it to dllimport. Maybe something like this in public headers:

#ifndef LIBIMOBILEDEVICE_API
#   ifdef WIN32
#      define LIBIMOBILEDEVICE_API __declspec( dllimport )
#   else
#      define LIBIMOBILEDEVICE_API
#   endif //WIN32
#endif //LIBIMOBILEDEVICE_API 

tihmstar avatar Nov 27 '23 22:11 tihmstar

The other solution is so use .def files on Windows and symbols files for GNU projects. Cleaner IMO.

neheb avatar Nov 28 '23 00:11 neheb

The other solution is so use .def files on Windows and symbols files for GNU projects. Cleaner IMO.

not really, because those files are compiler dependant, so you will need to generate multiple files for compilers that supports it, and what will you do for those ,that don't support it?

mexmer avatar Nov 28 '23 08:11 mexmer

don't apply? It's supported by at least MSVC, GCC, and Clang.

neheb avatar Nov 29 '23 00:11 neheb