Skip to content

Conversation

pluris
Copy link
Contributor

@pluris pluris commented Sep 27, 2023

A function to validate file descriptor in c++ land has been added.
This is a C++ implementation of getValidatedFd() in js.

Refs: #49898, #49880

@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 Sep 27, 2023
@pluris pluris changed the title src: add GetValidatedFd() in node_file src: add GetValidatedFd() to node_file Sep 27, 2023
@pluris pluris force-pushed the feat/get_validated_fd branch from ed908e6 to b35894e Compare September 27, 2023 17:11
@mscdex
Copy link
Contributor

mscdex commented Sep 27, 2023

I think we should only be adding code that is currently used somewhere. I get you have other PRs that use this function, but it would be better to just wait for one of them to land first and then rebase the other.

@pluris
Copy link
Contributor Author

pluris commented Sep 27, 2023

@mscdex
Thank you for your comment. I will wait for the PR in which this function is used to be landed. I will close this PR.

@pluris pluris closed this Sep 27, 2023
@pluris pluris deleted the feat/get_validated_fd branch November 8, 2023 02:24
@anonrig
Copy link
Member

anonrig commented Nov 30, 2023

Hey @pluris, Are you working on this? I'd like to continue on the work on node:fs

@pluris
Copy link
Contributor Author

pluris commented Nov 30, 2023

@anonrig No, I'm not working on it. I think it would be better for you to do it.

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.

4 participants