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

report: fix recent coverity warning. #48954

Closed
wants to merge 2 commits into from
Closed

Conversation

mhdawson
Copy link
Member

Fix warning about dereferencing null env

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. report Issues and PRs related to process.report. labels Jul 28, 2023
@mhdawson
Copy link
Member Author

This is the coverity report:

// External function to trigger a report, writing to file.
 945std::string TriggerNodeReport(Isolate* isolate,
 946                              const char* message,
 947                              const char* trigger,
 948                              const std::string& name,
 949                              Local<Value> error) {
    	1. assign_zero: Assigning: env = NULL.
 950  Environment* env = nullptr;
    	2. Condition isolate != NULL, taking false branch.
 951  if (isolate != nullptr) {
 952    env = Environment::GetCurrent(isolate);
 953  }
    	
CID 321974 (#1 of 1): Explicit null dereferenced (FORWARD_NULL)
3. var_deref_model: Passing null pointer env to TriggerNodeReport, which dereferences it. [[show details](https://scan9.scan.coverity.com/eventId=9118704-3&modelId=9118704-0&fileInstanceId=117525487&filePath=%2Fsrc%2Fnode_report.cc&fileStart=849&fileEnd=942)]
 954  return TriggerNodeReport(isolate, env, message, trigger, name, error);
 955}

@mhdawson
Copy link
Member Author

Wrapping in a if env != nullptr seemed consistent with an existing similar case a few lines lower -

if (env != nullptr) {

Fix warning about dereferencing null env

Signed-off-by: Michael Dawson <mdawson@devrus.com>
@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 3, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 3, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2023
@nodejs-github-bot
Copy link
Collaborator

mhdawson added a commit that referenced this pull request Aug 15, 2023
Fix warning about dereferencing null env

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48954
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@mhdawson
Copy link
Member Author

Landed in 3d5e7cd

@mhdawson mhdawson closed this Aug 15, 2023
UlisesGascon pushed a commit that referenced this pull request Sep 10, 2023
Fix warning about dereferencing null env

Signed-off-by: Michael Dawson <mdawson@devrus.com>

PR-URL: #48954
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@UlisesGascon UlisesGascon mentioned this pull request Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. report Issues and PRs related to process.report.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants