#523 explicit __cdecl for Microsoft Compilers: malloc/realloc/free

[Arno Schoedl]

The Microsoft Compiler allows to change the standard calling convention. Of course, even if you do, malloc/free/etc. keep their __cdecl. So the function pointer must be decorated:

#if defined(_MSC_VER)
#define __CDECL __cdecl
#else
#define __CDECL
#endif

typedef struct {
  void *(__CDECL *malloc_fcn)(size_t size);
  void *(__CDECL *realloc_fcn)(void *ptr, size_t size);
  void (__CDECL *free_fcn)(void *ptr);
} XML_Memory_Handling_Suite;

[Sebastian Pipping]

I guess that is relevant if you want to put pointers to the original malloc/realloc/free functions in there but not for self-made substitutes?
If we change that calling convention we break ABI to my understanding. Also I assume that people would rather use this feature with functions other than the originals and if they tried, they would have noticed. Making a trivial wrapper would work as workaround, I suppose.
To summarize, I believe this can be closed as "good enough" or "not worth the ABI break". What do you think?

[Sebastian Pipping]

Related: https://sourceforge.net/p/expat/feature-requests/1/

[Arno Schoedl]

It has been a while, but if I remember correctly, expat does not compile when the compiler switch is set. I do not feel strongly about how it should be solved, but expat should at least compile with the stdcall switch. The users who do not replace the default allocation functions are the least sophisticated ones. We shouldn't make their life harder. A default wrapper is fine, too, if you like that better.

[Arno Schoedl]

ABI compatibility only breaks if the compiler switch is set. Without it, cdecl is implicit and adding it explicitly makes no difference.

[Sebastian Pipping]

Hi Arno,

but expat should at least compile with the stdcall switch

With that said, I'm starting to understand now that we have these lines in Expat and they are the trouble ones:

mtemp->malloc_fcn = malloc;
mtemp->realloc_fcn = realloc;
mtemp->free_fcn = free;

I guess if we did a trick like

#if defined(_MSC_VER)
static void * _EXPAT_malloc(size_t size) { return malloc(size); }
#else
# define _EXPAT_malloc malloc
#endif

[..]

mtemp->malloc_fcn = _EXPAT_malloc;

it would work with either calling convention at the same time, assuming that MSVC knows that original malloc is always decl. What do you think?

Btw is this calling convetion switch still a thing with Visual Studio 2003 and alter? I'm asking because Visual Studio 6.0 is not supported any more.

Best, Sebastian

[Sebastian Pipping]

A quick update: I searched and found MSDN documention on calling conventions now [1], found how to integrate an stdcall switch to CMake build systems for debugging [2], was then able to reproduce the problem on AppVeyor [3], tried that wrapper idea mentioned above (patch attached) and ran into more errors related to stdcall then [4]. So my impression is that cdecl is the only supported default calling convention at the moment and to support stdcall would need a bit more. Is that something you would like to work on and make a pull request on GitHub?

Best, Sebastian

[1] https://msdn.microsoft.com/en-us/library/46t77ak2(v=vs.71).aspx
[2] https://github.com/libgit2/libgit2/blob/master/CMakeLists.txt#L160
[3] https://ci.appveyor.com/project/hartwork/libexpat/build/libexpat-32/job/714xio5af1x22aoc#L136
[4] https://ci.appveyor.com/project/hartwork/libexpat/build/libexpat-33/job/3lc9nj39i7ndfrei