Thank you! Aleksey
On 5/16/17 1:55 PM, Roumen Petrov wrote: > Hi Aleksey, > > Aleksey Sanin wrote: >> Roumen (and anyone else who has experience with MinGW environment), >> >> There is a pull request to make changes to MinGW build. Unfortunately, >> I have no experience on this platform and would love to get your >> feedback on whether this is the correct change or not: >> >> https://github.com/lsh123/xmlsec/pull/115/files > > My recommendation is to reject the request for the reasons described below: > > > a) src/gcrypt/app.c > Not mingw* specific. > What is wrong with long cast for printf %ld flag? > > > b) tests/testrun.sh > Wrong . Not mingw* specific. > mingw* produce native binaries that use OS functionality . > priv_key_suffix is related to OS (mis)functionality. Definitely is > not related to mingw GNU compiler. It is not compile time flag. It is > runtime flag! > > If I remember well it is hack for MS crypto store for certain version of > operation system. > Perhaps logic could be improved to be forward compatible. I mean > something like this "if OS version >= Win-XP then use methodA else > methodB". > > For example runtime in above case mean build is on win2k and using the > same build tree run tests on Win-XP, Win7 ... . > > > c) > - # To avoid problem with loading of a shared library (dlopen or > - # equivalent) at run time on some platforms we need to link > - # everything statically (it works without hack on 9x and under > - # emulation; on nt 5.x (w2k,xp) the error is 998: "Invalid > - # access to memory location"). > > I guess that above comment describes issue clearly. > If on OS version >= Win-7 issue does not exist then fine. May be issue > was specific to 32-bit MS OS and does not exist for 64 bit . > Unfortunately today I cannot test xmlsec in native environment. > > If I remember well Microsoft loader fail to load some shared > libraries(dll) dynamically due to limited functionality (failure in > initialization of some global variables(?)). > > So removing enable_static_linking="yes" and enable_crypto_dl="no" needs > addition clarification (testing on native platform) and today I cannot > help. > => Must be separate request. > > > d) shared vs static build and definition of XMLSEC_STATIC > Define of -DXMLSEC_STATIC=1 in configure is just plain wrong. > > First I would like to point that xmlsec uses libtool for builds in unix > like environment > The build is controlled by libtool flags exported to configure > --enable-shared[=PKGS] build shared libraries [default=yes] > --enable-static[=PKGS] build static libraries [default=yes] > > During make phase libtool shows one command but actually performs two > compilation - one for static and one for shared. > > > Logic should be in common header something like following > #ifdef PIE > # defines for shared build > #else > # defines for static build > #endif > > Remark : libtool pass -DPIE if compilation is for shared library > > Note I have no issue with current build system and I'm not sure why is > necessary to change code. > May be mingw* complier is old and issue is for mingw* gcc >= 6*. > > > e) extra > Out of scope but you may want to update libtool macros in configure script > Please find attached file "0004-remove-obsolete-AC_PROG_LIBTOOL.patch" > > AC_PROG_LIBTOOL is defined for backward compatibility in libtool 2+ and > LT_INIT([win32-dll]) is enough. > > > Regards > Roumen Petrov > > > _______________________________________________ > xmlsec mailing list > [email protected] > http://www.aleksey.com/mailman/listinfo/xmlsec > _______________________________________________ xmlsec mailing list [email protected] http://www.aleksey.com/mailman/listinfo/xmlsec
