GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — src: fix some recently introduced coverity issues by mhdawson · Pull Request #47240 · nodejs/node

Conversation

@mhdawson

@mhdawson mhdawson commented Mar 23, 2023

Copy link
Copy Markdown
Member

No description provided.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@mhdawson
src: fix some recently introduced coverity issues
3ae2db2
Signed-off-by: Michael Dawson <[email protected]>
@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. labels Mar 23, 2023
richardlau
richardlau approved these changes Mar 23, 2023
View reviewed changes
@mhdawson

mhdawson commented Mar 23, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Coverity reports

src/dataqueue/queue.cc
public:
 788  static std::unique_ptr<FdEntry> Create(Environment* env, Local<Value> path) {
 789    // We're only going to create the FdEntry if the file exists.
    	1. var_decl: Declaring variable req without initializer.
 790    uv_fs_t req;
 791    auto cleanup = OnScopeLeave([&] { uv_fs_req_cleanup(&req); });
 792
 793    auto buf = std::make_shared<BufferValue>(env->isolate(), path);
    	2. Condition uv_fs_stat(NULL, &req, buf->out(), NULL) < 0, taking false branch.
 794    if (uv_fs_stat(nullptr, &req, buf->out(), nullptr) < 0) return nullptr;
 795
    	
CID 310020 (#1 of 1): Uninitialized scalar variable (UNINIT)
3. uninit_use_in_call: Using uninitialized value req.statbuf when calling make_unique.
 796    return std::make_unique<FdEntry>(
 797        env, std::move(buf), req.statbuf, 0, req.statbuf.st_size);
 798  }

static bool CheckModified(FdEntry* entry, int fd) {
    	1. var_decl: Declaring variable req without initializer.
 852    uv_fs_t req;
 853    auto cleanup = OnScopeLeave([&] { uv_fs_req_cleanup(&req); });
 854    // TODO(jasnell): Note the use of a sync fs call here is a bit unfortunate.
 855    // Doing this asynchronously creates a bit of a race condition tho, a file
 856    // could be unmodified when we call the operation but then by the time the
 857    // async callback is triggered to give us that answer the file is modified.
 858    // While such silliness is still possible here, the sync call at least makes
 859    // it less likely to hit the race.
    	2. Condition uv_fs_fstat(NULL, &req, fd, NULL) < 0, taking false branch.
 860    if (uv_fs_fstat(nullptr, &req, fd, nullptr) < 0) return true;
    	3. uninit_use_in_call: Using uninitialized value req.statbuf.st_size when calling is_modified. [[show details](https://scan9.scan.coverity.com/eventId=9011842-3&modelId=9011842-0&fileInstanceId=111313726&filePath=%2Fsrc%2Fdataqueue%2Fqueue.cc&fileStart=846&fileEnd=849)]
    	
CID 310018 (#1-2 of 2): Uninitialized scalar variable (UNINIT)
4. uninit_use_in_call: Using uninitialized value req.statbuf.st_mtim.tv_nsec when calling is_modified. [[show details](https://scan9.scan.coverity.com/eventId=9011842-6&modelId=9011842-1&fileInstanceId=111313726&filePath=%2Fsrc%2Fdataqueue%2Fqueue.cc&fileStart=846&fileEnd=849)]
 861    return entry->is_modified(req.statbuf);
 862  }

test/embedding/embedtest.cc

    } else {
   	5. var_decl: Declaring variable req without initializer.
 78      uv_fs_t req;
 79      int statret = uv_fs_stat(nullptr, &req, filename, nullptr);
   	6. Condition statret == 0, taking true branch.
 80      assert(statret == 0);
   	
CID 307777 (#1 of 1): Uninitialized scalar variable (UNINIT)
7. uninit_use: Using uninitialized value req.statbuf.st_size.
 81      size_t filesize = req.statbuf.st_size;
 82      uv_fs_req_cleanup(&req);
 83
 84      std::vector<char> vec(filesize);
 85      size_t read = fread(vec.data(), filesize, 1, fp);
 86      assert(read == 1);
 87      snapshot = node::EmbedderSnapshotData::FromBlob(vec);
 88    }
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhdawson

mhdawson commented Mar 23, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Not sure why it would have failed for coverage and not for my local make test linux run, will have to look at it tomorrow.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhdawson
squash: update initialization to avoid warnings
d68c62e
Signed-off-by: Michael Dawson <[email protected]>
@richardlau

richardlau commented Mar 23, 2023

Copy link
Copy Markdown
Member

Not sure why it would have failed for coverage and not for my local make test linux run, will have to look at it tomorrow.

My guess would be either compiler level and/or the --error-on-warn configure flag.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhdawson
squash: run make format-cpp
6e496bd
Signed-off-by: Michael Dawson <[email protected]>
@mhdawson

mhdawson commented Mar 23, 2023

Copy link
Copy Markdown
Member Author

Updated to initialize in another way, lets see if the compilers are happy with it across platforms. Also compiled ok locally.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhdawson mhdawson added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 24, 2023
@mhdawson

mhdawson commented Mar 24, 2023

Copy link
Copy Markdown
Member Author

Requesting ci to check across all platforms

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Mar 24, 2023
@nodejs-github-bot

nodejs-github-bot commented Mar 24, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/50588/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/50600/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/50601/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

lpinca
lpinca approved these changes Mar 25, 2023
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/50602/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/50604/

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhdawson

mhdawson commented Mar 28, 2023

Copy link
Copy Markdown
Member Author

Landed in fe449a2

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhdawson mhdawson closed this Mar 28, 2023
mhdawson added a commit that referenced this pull request Mar 28, 2023
@mhdawson
src: fix some recently introduced coverity issues
fe449a2
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #47240
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@github-actions github-actions Bot mentioned this pull request Mar 30, 2023
CI Reliability 2023-03-30 nodejs/reliability#534
Open
18 tasks
@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
2023-04-18, Version 20.0.0 (Current) #47381
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@mhdawson @RafaelGSS
src: fix some recently introduced coverity issues
f80697d
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #47240
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
v19.9.0 proposal #47441
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
@mhdawson @RafaelGSS
src: fix some recently introduced coverity issues
cb2a6fa
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #47240
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@mhdawson @RafaelGSS
src: fix some recently introduced coverity issues
d0a5e7e
Signed-off-by: Michael Dawson <[email protected]>

PR-URL: #47240
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@danielleadams danielleadams added the dont-land-on-v18.x label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lpinca lpinca lpinca approved these changes
@richardlau richardlau richardlau approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@mhdawson @richardlau @nodejs-github-bot @lpinca @danielleadams
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.