-
-
Notifications
You must be signed in to change notification settings - Fork 34.3k
src: use ToLocal() in napi_create_bigint_words #60165
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
Conversation
|
Review requested:
|
Replace ToLocalChecked() with MaybeLocal::ToLocal() for BigInt::NewFromWords. On failure, return napi_set_last_error(env, napi_generic_failure) to correctly propagate the error instead of risking a crash. This aligns with existing error-handling macros and avoids unchecked conversions.
46a882a to
3d03c6e
Compare
legendecas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CHECK_MAYBE_EMPTY_WITH_PREAMBLE should already handles the empty result from v8::BigInt::NewFromWords.
Could you clarify how this change is different from CHECK_MAYBE_EMPTY_WITH_PREAMBLE?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60165 +/- ##
==========================================
- Coverage 88.55% 88.54% -0.01%
==========================================
Files 704 704
Lines 208089 208091 +2
Branches 40017 40015 -2
==========================================
- Hits 184270 184262 -8
- Misses 15818 15861 +43
+ Partials 8001 7968 -33
🚀 New features to boost your workflow:
|
Thank you for clarifying, It seems I misunderstood. Just to confirm, is it correct to say that ToLocalChecked() is safe to use as long as the value has been validated beforehand?" |
Yes, it is corret. For cases, we would recommend |
Replace ToLocalChecked() with MaybeLocal::ToLocal() for BigInt::NewFromWords.
On failure, return napi_set_last_error(env, napi_generic_failure) to correctly propagate the error instead of risking a crash.
This aligns with existing error-handling macros and avoids unchecked conversions.