Skip to content

src: initialize FSReqWrapSync in path that uses it#56613

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
targos:coverity_node_file
Jan 19, 2025
Merged

src: initialize FSReqWrapSync in path that uses it#56613
nodejs-github-bot merged 1 commit intonodejs:mainfrom
targos:coverity_node_file

Conversation

@targos
Copy link
Member

@targos targos commented Jan 15, 2025

The goal is to fix this error reported by Coverity:

image

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jan 15, 2025
@targos
Copy link
Member Author

targos commented Jan 15, 2025

Well it looks like this is breaking some tests. I'll come back to it when I'm on my Mac

@codecov
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.19%. Comparing base (c8df98d) to head (d590d06).
Report is 20 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #56613      +/-   ##
==========================================
- Coverage   89.20%   89.19%   -0.01%     
==========================================
  Files         662      662              
  Lines      191816   191819       +3     
  Branches    36926    36921       -5     
==========================================
- Hits       171102   171090      -12     
- Misses      13559    13571      +12     
- Partials     7155     7158       +3     
Files with missing lines Coverage Δ
src/node_file.cc 77.23% <100.00%> (ø)

... and 35 files with indirect coverage changes

@targos targos force-pushed the coverity_node_file branch from 2b07c23 to d590d06 Compare January 15, 2025 17:26
@targos
Copy link
Member Author

targos commented Jan 15, 2025

Apparently it's important that the stack_buffer still lives after use, so I updated to only change the variable that was flagged by Coverity.

@targos targos changed the title src: initialize variables in paths that use them src: initialize FSReqWrapSync in path that uses it Jan 15, 2025
@targos
Copy link
Member Author

targos commented Jan 16, 2025

CI looks good now. Can I have a new review?

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 16, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 16, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jan 16, 2025

@jasnell jasnell added the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 19, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 19, 2025
@nodejs-github-bot nodejs-github-bot merged commit 322056d into nodejs:main Jan 19, 2025
61 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 322056d

@targos targos deleted the coverity_node_file branch January 19, 2025 16:48
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++. fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants