Add DLL exports. #223

1. 
   evoskuil commented at 10:34 pm on February 23, 2015: contributor
2. 
   sipa commented at 10:34 am on February 24, 2015: contributor

   @theuni Comments?
3. gmaxwell added this to the milestone initial release on Aug 31, 2015
4. Add visibility hidden attribute to non-MSVC statics, rebase. 36daecf5c6
5. in include/secp256k1.h: in d29e19bebe outdated

   53+#  endif
   54+# endif
   55+
   56+# if defined SECP256K1_STATIC
   57+#  define SECP256K1_API
   58+# elif defined SECP256K1_DLL
   

   ---

   ---

   

   chfast commented at 2:16 pm on September 14, 2015:

   CMake has a convention to add macro MyLibrary_EXPORTS when building a DLL. Is it possible to rename SECP256K1_DLL to SECP256K1_EXPORTS?

   ---

   

   evoskuil commented at 6:05 pm on September 14, 2015:

   Certainly, the symbol is otherwise arbitrary.

   ---

   

   theuni commented at 10:45 pm on September 14, 2015:

   Sounds good to me.

   I think this should be __attribute__ ((visibility ("hidden"))) in the SECP256K1_STATIC case though, since it wouldn’t need to be externally visible

   ---

   

   evoskuil commented at 11:06 pm on September 14, 2015:

   Good idea…

    0    define SECP256K1_HELPER_DLL_EXPORT __declspec(dllexport)
    1else
    2    if __GNUC__ >= 4
    3        define SECP256K1_HELPER_DLL_IMPORT __attribute__ ((visibility ("default")))
    4        define SECP256K1_HELPER_DLL_EXPORT __attribute__ ((visibility ("default")))
    5        define SECP256K1_HELPER_STATIC __attribute__ ((visibility ("hidden")))
    6    else
    7        define SECP256K1_HELPER_DLL_IMPORT
    8        define SECP256K1_HELPER_DLL_EXPORT
    9        define SECP256K1_HELPER_STATIC
   10    endif
   11endif
   12
   13if defined SECP256K1_STATIC
   14    define SECP256K1_API SECP256K1_HELPER_STATIC
   15elif defined SECP256K1_DLL
   

6. in include/secp256k1.h: in d29e19bebe outdated

   39@@ -40,6 +40,26 @@ extern "C" {
   40 #  define SECP256K1_ARG_NONNULL(_x)
   41 # endif
   42 
   43+# if defined _MSC_VER || defined __CYGWIN__
   44+#  define SECP256K1_HELPER_DLL_IMPORT __declspec(dllimport)
   

   ---

   ---

   

   chfast commented at 10:49 pm on September 14, 2015:

   dllimport is only useful in case of global variables and C++ classes, not in case of functions. Removing dllimport support would simplify the patch.

   ---

   

   evoskuil commented at 11:32 pm on September 14, 2015:

   I’m not sure this is the case. This is an old reference, but I have to assume it still holds:

   You do not need to use __declspec(dllimport) for your code to compile correctly, but doing so allows the compiler to generate better code.

   https://msdn.microsoft.com/en-us/library/aa271769(v=vs.60).aspx

   ---

   

   chfast commented at 8:50 pm on September 16, 2015:

   @evoskuil, you are right. I’ve checked the Visual Studio 2013 and it generates better code when dllimport is used (as described in the article).
7. evoskuil force-pushed on Sep 14, 2015
8. Rename static export helpers and public export symbol, export extern vars. 5c571f3f72
9. Export ecdh, recovery and schnorr APIs. 58fd107baa
10. in include/secp256k1.h: in 58fd107baa

    152+# elif defined SECP256K1_EXPORTS
    153+#  define SECP256K1_API SECP256K1_HELPER_DLL_EXPORT
    154+# else
    155+#  define SECP256K1_API SECP256K1_HELPER_DLL_IMPORT
    156+# endif
    157+
    

    ---

    ---

    

    evoskuil commented at 0:14 am on September 15, 2015:

    The above matches closely the GCC example (bottom of page). There is deviation in the public symbols SECP256K1_STATIC and SECP256K1_EXPORTS. The latter is modified for better CMake integration (see comment by @chfast). WRT the former I consider “static” more explicit than “local” and therefore better for publication.
11. in src/secp256k1.c: in 58fd107baa

    253@@ -254,7 +254,10 @@ static int nonce_function_rfc6979(unsigned char *nonce32, const unsigned char *m
    254    return 1;
    255 }
    256 
    257+SECP256K1_API
    258 const secp256k1_nonce_function_t secp256k1_nonce_function_rfc6979 = nonce_function_rfc6979;
    259+
    260+SECP256K1_API
    261 const secp256k1_nonce_function_t secp256k1_nonce_function_default = nonce_function_rfc6979;
    

    ---

    ---

    

    evoskuil commented at 0:16 am on September 15, 2015:

    I have not had a chance to verify these two lines, but according to documentation it appears correct for both VC and GCC.
12. theuni cross-referenced this on Sep 21, 2015 from issue Use explicit symbol visibility. by gmaxwell
13. 
    gmaxwell commented at 0:12 am on September 23, 2015: contributor

    Please take a look at #314
14. 
    evoskuil commented at 2:20 am on September 23, 2015: contributor

    I prefer the readability of the gcc example layout, and the use of dllimport, but not a big deal in either case.
15. 
    chfast commented at 7:32 am on September 23, 2015: none

    @gmaxwell #314 looks like a subset of this one. With the except GCC visibility is not set to hidden by default here.
16. 
    sipa commented at 1:49 pm on September 23, 2015: contributor

    This defines SECP256K1_STATIC and SECP256K1_EXPORT, but they aren’t being set anywhere?

    If in #314, in the non-SECP256K1_BUILD branch for windows, declspec(dllimport) is set rather than nothing, do we get the best of both worlds?

17. 
    sipa commented at 1:51 pm on September 23, 2015: contributor

    I also think that this fails to set dllexport/import when compiling with MinGW?
18. evoskuil cross-referenced this on Sep 24, 2015 from issue Use explicit symbol visibility. by gmaxwell
19. 
    sipa commented at 7:37 pm on September 25, 2015: contributor

    Superseded by #314.
20. sipa closed this on Sep 25, 2015

    ---

Contributors
evoskuil sipa chfast theuni gmaxwell

Milestone
stable release (1.0.0-rc.1)