ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all \u003cpre\u003e\u003ccode\u003e blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks"); } } catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); } })(); (function(){ try { var __m = "github.com"; var __re = new RegExp('^' + "github\\.com" + '
Skip to content

ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length \u003e 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

ffi: throw on missing memory helper arguments - #65500

Open
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments
Open

ffi: throw on missing memory helper arguments#65500
soulee-dev wants to merge 1 commit into
nodejs:mainfrom
soulee-dev:ffi-throw-on-missing-arguments

Conversation

@soulee-dev

Copy link
Copy Markdown
Contributor

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return undefined instead of throwing when a required argument is omitted, so a call that read or wrote nothing cannot be told apart from one that read a zero byte.

GetValidatedPointerAddress() and GetValidatedSize() already reject the same argument when it is passed explicitly as undefined. The args.Length() test in front of them short-circuits the call and returns Nothing without scheduling an exception. These six are the only tests in src/ where args.Length() can skip a call that throws; the only other Length() tests that guard a call at all guard Buffer::HasInstance(), which cannot throw.

$ # before
$ node --experimental-ffi -p "require('node:ffi').getUint8()"undefined
$ # after
$ node --experimental-ffi -p "require('node:ffi').getUint8()"TypeError: The pointer must be a bigint

Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for an out-of-range index, which is exactly the value these validators reject, so each missing argument now produces the error its explicit undefined counterpart produces. The tests that guard an explicit throw, such as the ones in ToString() and GetRawPointer(), are left alone.

ExportBytes() carried the same two tests. They are unreachable through the public API because exportBytes is not exported and its three callers all validate len in JavaScript first, but they are the same shape.

No documentation change is needed: doc/api/ffi.md already lists these arguments as required.

Fixes: #65499
Refs: #62072
Refs: #62858

ffi.getInt8() through ffi.getFloat64(), ffi.setInt8() through
ffi.setFloat64(), ffi.toBuffer() and ffi.toArrayBuffer() return
undefined instead of throwing when a required argument is omitted, so a
call that read or wrote nothing cannot be told apart from one that read
a zero byte. All 22 helpers behave this way.
GetValidatedPointerAddress() and GetValidatedSize() already reject the
same argument when it is passed explicitly as undefined. The
args.Length() test in front of them short-circuits the call and returns
Nothing without scheduling an exception. These six are the only tests in
src/ where args.Length() can skip a call that throws; the only other
Length() tests that guard a call at all guard Buffer::HasInstance(),
which cannot throw. The remaining tests in this file guard an inline
predicate and throw in the branch, which is why setUint8(ptr) reports
"Expected an offset argument" while setUint8() reports nothing at all.
Drop those tests. FunctionCallbackInfo::operator[] returns Undefined for
an out-of-range index, which is exactly the value these validators
reject, so each missing argument now produces the error its explicit
undefined counterpart produces. The documentation already describes this
behavior: the signatures are ffi.getInt8(pointer[, offset]),
ffi.setInt8(pointer, offset, value) and
ffi.toBuffer(pointer, length[, copy]), and the getters are documented to
return a number or a bigint.
ExportBytes() carried the same two tests. They are unreachable through
the public API because exportBytes is not exported and its three callers
all validate len in JavaScript first, but they are the same shape.
Signed-off-by: Soul Lee <alus20x@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/ffi

@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. labels Aug 23, 2026
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.12%. Comparing base (a48e33f) to head (8926b4d).
⚠️ Report is 27 commits behind head on main.

Files with missing linesPatch %Lines
src/ffi/data.cc66.66%0 Missing and 2 partials ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #65500 +/- ##
========================================
Coverage 90.12% 90.12% ========================================
Files 751 751 Lines 252425 252589 +164 Branches 47471 47535 +64 ========================================
+ Hits 227504 227655 +151 - Misses 16216 16239 +23 + Partials 8705 8695 -10 
Files with missing linesCoverage Δ
src/ffi/data.cc77.55% <66.66%> (+1.83%)⬆️

... and 61 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.

Sign up for freeto 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-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ffi: memory helpers return undefined instead of throwing when a required argument is omitted

2 participants

@soulee-dev@nodejs-github-bot