Skip to content

src: bring permissions macros in line with general C/C++ standards - #60053

Closed
addaleax wants to merge 1 commit into
nodejs:mainfrom
addaleax:permissions-macros
Closed

src: bring permissions macros in line with general C/C++ standards#60053
addaleax wants to merge 1 commit into
nodejs:mainfrom
addaleax:permissions-macros

Conversation

@addaleax

Copy link
Copy Markdown
Member

Specifically, avoid the hazard of unintentionally evaluating an argument multiple times during macro expansion, and do not assume the available of particular namespaces in the current scope.

Specifically, avoid the hazard of unintentionally evaluating
an argument multiple times during macro expansion, and do not
assume the available of particular namespaces in the current scope.
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/security-wg

@nodejs-github-botnodejs-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 Sep 28, 2025
@addaleaxaddaleax added the permission Issues and PRs related to the Permission Model label Sep 28, 2025
@codecov

codecovBot commented Sep 28, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.46%. Comparing base (c6316f9) to head (2c7627c).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #60053 +/- ##
==========================================
+ Coverage 88.45% 88.46% +0.01% 
==========================================
Files 703 703 Lines 207546 207546 Branches 40011 40008 -3 ==========================================
+ Hits 183591 183614 +23 + Misses 15949 15940 -9 + Partials 8006 7992 -14 
Files with missing linesCoverage Δ
src/node_report.cc93.16% <ø> (ø)
src/permission/permission.h83.33% <ø> (ø)

... and 30 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@addaleaxaddaleax added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 28, 2025
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 28, 2025
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@RafaelGSSRafaelGSS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

addaleax added a commit that referenced this pull request Sep 30, 2025
Specifically, avoid the hazard of unintentionally evaluating
an argument multiple times during macro expansion, and do not
assume the available of particular namespaces in the current scope.
PR-URL: #60053
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
@addaleax

Copy link
Copy Markdown
MemberAuthor

Landed in 5d1adb8

RafaelGSS pushed a commit that referenced this pull request Oct 7, 2025
Specifically, avoid the hazard of unintentionally evaluating
an argument multiple times during macro expansion, and do not
assume the available of particular namespaces in the current scope.
PR-URL: #60053
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
aduh95 pushed a commit that referenced this pull request Oct 22, 2025
Specifically, avoid the hazard of unintentionally evaluating
an argument multiple times during macro expansion, and do not
assume the available of particular namespaces in the current scope.
PR-URL: #60053
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.permissionIssues and PRs related to the Permission ModelreportIssues and PRs related to process.report.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@addaleax@nodejs-github-bot@jasnell@RafaelGSS