ufairiya / mongoose

Automatically exported from code.google.com/p/mongoose
MIT License
0 stars 0 forks source link

Windows DLL needs stdcall declarations in types #81

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
In order to package mongoose.c as a Win32 dll for use with the c# bindings, the 
callbacks must 
be stdcall in order for the .net platform invoke infrastructure to work 
correctly. Various parts of 
MSDN note that a callback from C must be __stdcall to work with the delegate 
marshalling code.

So in mongoose.h:

typedef void (*mg_callback_t)(struct mg_connection *,
        const struct mg_request_info *info, void *user_data);

needs to read (when compiled with the microsoft compiler - I assume MingW has 
its own 
incantation):

typedef void (__stdcall *mg_callback_t)(struct mg_connection *,
        const struct mg_request_info *info, void *user_data);

I would propose  macro is added so that this callback type can be customised as 
needed:

typedef void (MGCALLBACK *mg_callback_t)(struct mg_connection *,
        const struct mg_request_info *info, void *user_data);

and MGCALLBACK can then be suitably defined for the Microsoft/MingW compilers.

Original issue reported on code.google.com by jhmcale...@gmail.com on 9 Aug 2009 at 11:45

GoogleCodeExporter commented 9 years ago

Original comment by valenok on 14 Aug 2009 at 8:00

GoogleCodeExporter commented 9 years ago
Submitted http://code.google.com/p/mongoose/source/detail?r=467

Instead of changing mongoose.h, I added /Gz compilation flag to the MSVC build.
This enables __stdcall as default calling convention.
Not very nice, but I rather prefer doing that instead of littering mongoose.h 
with 
windows specific (ugly) conditionals.

Original comment by valenok on 17 Aug 2009 at 9:28

GoogleCodeExporter commented 9 years ago
I would ask that you reconsider this fix.

Whilst the immediate cause is interop on Windows with C#, I do not believe such 
declarations are unique to Microsoft 
Windows. They can appear whenever the C language is asked to interop with non C 
subsystems, and I can think of other 
examples in embedded OS for things like this.

Of course, in the C-friendly unix world, I am unaware of them being needed. I 
was careful to suggest the macro be 
'MGCALLBACK' and not 'WINDOWSWART', anticipating the need to use this on 
embedded systems in the future.

/Gz is also sub-optimal. It means that combining mongoose.c with a .lib file 
which cannot (for whatever reason) be 
compiled with /Gz is impractical. I will have to maintain a personal fork of 
mongoose.h in this scenario, with all the 
disadvantages that entails.

Original comment by jhmcale...@gmail.com on 30 Aug 2009 at 11:06

GoogleCodeExporter commented 9 years ago
Unfortunately, it is only Windows that causes most trouble, including this mess 
with
calling convention. UNIX is way more friendly to the developer.

What I can see now is that my fix was not right. It is not fixing the problem 
you're
talking about, as you pointed out. You were talking about the callbacks, and I 
have
changed all exports. Do you have a diff for the .h change?

Original comment by valenok on 30 Aug 2009 at 12:13

GoogleCodeExporter commented 9 years ago
Unfortunately, I did not preserve the diff I created while I examined the 
mongoose c# bindings. These issues, 
and the instability that someone else has reported in #85 led me to abandon the 
code, and return to my custom 
(but ugly) CLR/C++ marshalling. Sorry.

For the record, I think your fix probably would work (it would make the 
mongoose dll correctly call in to c# with 
__stdcall), but it unfortunately breaks other things (like the ability to 
combine mongoose.c with other .libs in the 
same dll). 

Original comment by jhmcale...@gmail.com on 30 Aug 2009 at 5:31

GoogleCodeExporter commented 9 years ago
I have already reverted my "non-fix", and trying to play with cdecl/stdcall for 
the
callbacks. Thank you!

Original comment by valenok on 30 Aug 2009 at 6:59

GoogleCodeExporter commented 9 years ago
Issue 130 has been merged into this issue.

Original comment by valenok on 7 Sep 2010 at 7:20

GoogleCodeExporter commented 9 years ago
Fixed in latest sources.

Original comment by valenok on 7 Sep 2010 at 8:13