search

robertwb / issues-import-test

0 stars 0 forks source link

__stdcall define wrong for Windows platform #38

Closed robertwb closed 8 years ago

robertwb commented 8 years ago

Reported by JimKleckner on 23 May 2008 00:41 UTC The previous patch that fixed the redefinition of !__stdcall on Cygwin caused the Windows platform to fail to recognize the calling convention.

The attached patch adds a check for MS_WINDOWS as well before !__stdcall is turned off.

Migrated-From: http://trac.cython.org/ticket/12

robertwb commented 8 years ago

Comment by robertwb on 23 May 2008 02:55 UTC I'm not quite sure I understand--if MS_WINDOWS is defined, then isn't __stdcall defined as well?

robertwb commented 8 years ago

Comment by JimKleckner on 23 May 2008 03:23 UTC Correct.

That is why the stub for !__stdcall is protected by an

ifndef MS_WINDOWS

so that it won't blow it away in that case.

The issue is that Python.h doesn't include so that when the conditions are processed, !__stdcall will not yet be defined. In that case, we need to protect it with the platform define.

In the DllMain embedding I'm playing with, I explicitly include windows.h which then causes it to become defined. Without the protection of MS_WINDOWS, it has already been made into an empty string value.

robertwb commented 8 years ago

Comment by JimKleckner on 25 Jul 2008 16:27 UTC I just spent a couple of hours tracking down a linkage problem after updating my tool chain because I forgot about this patch that never got applied.

One can argue that this is major if you are trying to build something on Windoze because linkage fails.

Please either apply the patch or tell my why you won't.

robertwb commented 8 years ago

Comment by JimKleckner on 25 Jul 2008 17:22 UTC Attached is an updated patch that also defines _USE_MATH_DEFINES so that M_PI and such will get defined as with the standard math.h.

robertwb commented 8 years ago

Modified by robertwb on 3 Aug 2008 06:45 UTC

robertwb commented 8 years ago

Comment by robertwb on 15 Aug 2008 08:58 UTC Sorry it has taken so long, I don't have Windows to test this on, and no one has stepped up to do the same. (Also, it was marked as "minor" so it slipped under the radar for a while.)

Thank you for the patch, it's in now.

robertwb commented 8 years ago

Modified by robertwb on 19 Aug 2008 03:59 UTC