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

Add TRY_CATCH_CALL function check and remove Nan::MakeCallback #670

Closed
wants to merge 2 commits into from

Conversation

xbolshe
Copy link

@xbolshe xbolshe commented Jun 26, 2016

Signed-off-by: Eugene Bolshakov pub@relvarsoft.com

This patch fixes 2 issues:

  1. There is no check for a callback function presence in TRY_CATCH_CALL
  2. Remove Nan::MakeCallback due to MakeCallback is dangerous, not only or best way of calling callback; provide direct method nodejs/nan#284 and use Nan::Call

Signed-off-by: Eugene Bolshakov <pub@relvarsoft.com>
Signed-off-by: Eugene Bolshakov <pub@relvarsoft.com>
@xbolshe
Copy link
Author

xbolshe commented Jun 26, 2016

It is better to use Nan::Call due to v8 differences.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 86.276% when pulling eeb6d2b on xbolshe:master into f5dd456 on mapbox:master.

@springmeyer
Copy link
Contributor

@xbolshe - thank you for this fix. It looks like in #665 you also have a testcase that failed before this PR. Can you add that as well?

@xbolshe
Copy link
Author

xbolshe commented Oct 24, 2016

If you mean this post #665 (comment), it does not related with node-sqlite3. The issue was related with a MMX support on the target CPU (it does not support MMX). The issue was fixed after a compilation with disabling MMX.

@daniellockyer
Copy link
Member

This library has since switched away from NAN so this should no longer be relevant - closing for now 🙂

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

Successfully merging this pull request may close these issues.

4 participants