Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

winsqlite3.dll should use CallingConvention.StdCall #149

Closed
bricelam opened this issue Mar 15, 2017 · 3 comments
Closed

winsqlite3.dll should use CallingConvention.StdCall #149

bricelam opened this issue Mar 15, 2017 · 3 comments

Comments

@bricelam
Copy link
Contributor

bricelam commented Mar 15, 2017

Still working out the details on this one, but it sounds like the P/Invokes for winsqlite3.dll should use CallingConvention.StdCall instead of Cdecl.

cc @vrk7bp

@ericsink
Copy link
Owner

I'm a bit short on knowledge here, so I ask your patience with a possibly dumb question:

If the C# code is using the wrong calling convention for that native DLL, why does it work?

@bricelam
Copy link
Contributor Author

bricelam commented Mar 16, 2017

You'll only observe differences in a few corner cases. The two things that have been mentioned so far are:

  • You could get a corrupt stack when SQLite invokes a managed callback
  • Trying to debug the app crashes with a corrupt stack

ericsink added a commit that referenced this issue Apr 12, 2017
…build. leave all of them Cdecl except winsqlite3, which apparently needs to be StdCall. see #149
@ericsink
Copy link
Owner

OK, I have just pushed 1.1.3 to nuget, with the change to StdCall for winsqlite3. So I'm closing this as fixed.

That said, I'd be happier if I had a test case which failed with Cdecl. My test suite, which contains a number of cases where SQLite invokes a managed callback, passes with StdCall, but it also passed with Cdecl. So at the moment, I see no visible difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants