- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.4k
Make fstat work on file descriptor with no name #23058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Closed
      
        
      
            hoodmane
  wants to merge
  40
  commits into
  emscripten-core:main
from
hoodmane:anonymous-file-descriptors
  
      
      
   
      
    
  
     Closed
                    Changes from 9 commits
      Commits
    
    
            Show all changes
          
          
            40 commits
          
        
        Select commit
          Hold shift + click to select a range
      
      f3f36d5
              
                Make fstat work on file descriptor with no name
              
              
                hoodmane 260278f
              
                Fix node rawfs
              
              
                hoodmane f865bdb
              
                Cleanup
              
              
                hoodmane cac8b19
              
                Fix nodefs handling of unnamed file descriptors
              
              
                hoodmane cad0ddf
              
                Fix indentation
              
              
                hoodmane 8b1711e
              
                Apply review comments to test
              
              
                hoodmane c961354
              
                Fix flake8
              
              
                hoodmane bebfc6b
              
                Better argment names and comments
              
              
                hoodmane 7f4533e
              
                Fix ftruncate
              
              
                hoodmane 2c41571
              
                Fix directory fds for nameless file patch
              
              
                hoodmane 6a4dfc2
              
                Implement both truncate and ftruncate via truncateCommon
              
              
                hoodmane 9d0c79a
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane baadd84
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane 3aa358f
              
                Fix merge
              
              
                hoodmane 04192f0
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane 989cf22
              
                Refactor things a bit more
              
              
                hoodmane ad02402
              
                Change doStat to writeStat
              
              
                hoodmane 84bcf30
              
                Fix tests
              
              
                hoodmane ba35730
              
                Fix reference error
              
              
                hoodmane 4e67e7f
              
                Fix test
              
              
                hoodmane 9399402
              
                Declare stream variable
              
              
                hoodmane 7593dc1
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane efff804
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane 20de480
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane e83ac75
              
                Remove incorrect comment
              
              
                hoodmane b509358
              
                Fix merge
              
              
                hoodmane 8c556c8
              
                Address review comment
              
              
                hoodmane 4ed8004
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane bdfca83
              
                Fix test
              
              
                hoodmane 4fdc681
              
                Fix test
              
              
                hoodmane ddb3fab
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane bfd07c7
              
                Fix test
              
              
                hoodmane 5d4c6d2
              
                Merge branch 'main' into anonymous-file-descriptors
              
              
                hoodmane ac100ab
              
                Move stream ops access to streamGetAttr and streamSetAttr
              
              
                hoodmane d1f1962
              
                Fix syntax error
              
              
                hoodmane 02e0b69
              
                Slightly shorter
              
              
                hoodmane 910b824
              
                Slightly shorter
              
              
                hoodmane 37f8ee9
              
                Some fixes
              
              
                hoodmane 5eef321
              
                Fix more
              
              
                hoodmane ad6f42a
              
                More fixes
              
              
                hoodmane File filter
Filter by extension
Conversations
          Failed to load comments.   
        
        
          
      Loading
        
  Jump to
        
          Jump to file
        
      
      
          Failed to load files.   
        
        
          
      Loading
        
  Diff view
Diff view
There are no files selected for viewing
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              
              | Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| #include <fcntl.h> | ||
| #include <unistd.h> | ||
| #include <sys/stat.h> | ||
| #include <assert.h> | ||
| #include "stdio.h" | ||
|  | ||
| #ifdef __EMSCRIPTEN__ | ||
| #include <emscripten.h> | ||
| #endif | ||
|  | ||
| void makedir(const char *dir) { | ||
| int rtn = mkdir(dir, 0777); | ||
| assert(rtn == 0); | ||
| } | ||
|  | ||
| void changedir(const char *dir) { | ||
| int rtn = chdir(dir); | ||
| assert(rtn == 0); | ||
| } | ||
|  | ||
| void setup() { | ||
| #if defined(__EMSCRIPTEN__) && defined(NODEFS) | ||
| makedir("working"); | ||
| EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working')); | ||
| changedir("working"); | ||
| #endif | ||
| } | ||
|         
                  hoodmane marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||
|  | ||
|  | ||
| int main() { | ||
| setup(); | ||
| int fd = open("file.txt", O_RDWR | O_CREAT, 0666); | ||
| unlink("file.txt"); | ||
| int res; | ||
| struct stat buf; | ||
| res = fstat(fd, &buf); | ||
| assert(res == 0); | ||
| assert(buf.st_atime > 1000000000); | ||
| res = fchmod(fd, 0777); | ||
| assert(res == 0); | ||
| res = ftruncate(fd, 10); | ||
| assert(res == 0); | ||
| printf("success\n"); | ||
| } | ||
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
              | Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|  | @@ -5921,6 +5921,20 @@ def test_fs_64bit(self): | |||||
| self.set_setting('FORCE_FILESYSTEM') | ||||||
| self.do_runf('fs/test_64bit.c', 'success') | ||||||
|  | ||||||
| @requires_node | ||||||
| @parameterized({ | ||||||
| '': ([],), | ||||||
| 'nodefs': (['-DNODEFS', '-lnodefs.js'],), | ||||||
| 'noderawfs': (['-sNODERAWFS'],), | ||||||
| }) | ||||||
|         
                  hoodmane marked this conversation as resolved.
              Outdated
          
            Show resolved
            Hide resolved | ||||||
| def test_fs_stat_unnamed_file_descriptor(self, args): | ||||||
|          | ||||||
| def test_fs_stat_unnamed_file_descriptor(self, args): | |
| def test_fs_stat_unnamed_file_descriptor(self): | 
      
      Oops, something went wrong.
        
    
  
  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.
  
    
  
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a common pattern where we try to do an operation first on the stream and then on the node if it fails?
Do we need the fallback?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well we have a map from file descriptors to streams, and a map from paths to nodes. Each stream has a reference to its node. The difficulty is that not every node has a valid path.
Most file systems store all relevant information about a node on the node itself. However, in the nodefs, each stream has a distinct native file descriptor. In the case where the node has no valid path, the only way to talk to the native object is with
fstatand friends using the native file descriptor. So in that case we need thesestream_ops. For every other FS, it's sufficient to havesetattron thenode_ops.Also, we do need the
node_opscase because if we callstatinstead offstatthen we have a path but no file descriptor. We couldopenthe path to get the file descriptor and thenstatit and close it. But we're better off just having a separate handler for gettattr/setattr on a path/node than on a file descriptor/stream. But only nodefs seems to need it.