-
Notifications
You must be signed in to change notification settings - Fork 459
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
Fix/nullptr error message #228
Conversation
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.
LGTM assuming we don't have any other warnings/from existing warnings/errors from the static analysis.
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.
@mhdawson Prefast is clean but there are regular compiler warnings relating to lossy implicit conversions to double. Only affects the unit test code, not the main source code itself. |
Enabled VC++ static analysis PR-URL: #228 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Landed as faf19c4 |
Enabled VC++ static analysis PR-URL: nodejs/node-addon-api#228 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Enabled VC++ static analysis PR-URL: nodejs/node-addon-api#228 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Enabled VC++ static analysis PR-URL: nodejs/node-addon-api#228 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Enabled VC++ static analysis PR-URL: nodejs/node-addon-api#228 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Hitesh Kanwathirtha <hiteshk@microsoft.com>
Fixes #224.
The Napi::Error constructor was initializing the
std::string _message
private member variable tonullptr
. As a class, std::string automatically initializes to the equivalent of a blank string._message
isn't a pointer; assigning a pointer literal (nullptr) to it causes static analysis warnings.I've also enabled Visual C++ Static Analysis (Prefast) in the test suite to detect these sorts of problems. If that isn't desirable let me know and I'll back out that part of the PR.