-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
feat(nodefs): mmap and msync #10669
feat(nodefs): mmap and msync #10669
Conversation
src/library_nodefs.js
Outdated
throw new FS.ErrnoError({{{ cDefine('ENODEV') }}}); | ||
} | ||
var ptr = _malloc(length); | ||
NODEFS.stream_ops.read(stream, buffer, ptr + offset , length, position); |
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.
extra space before one of the ,
s
} | ||
var ptr = _malloc(length); | ||
NODEFS.stream_ops.read(stream, buffer, ptr + offset , length, position); | ||
|
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.
comparing to MEMFS, offset
is not used there, but you add it here - is that a bug there, or here? If we aren't sure, an assertion that the offset is 0 might be good enough for now.
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.
not sure actually, so I've added the assertion as you suggest.
add implementation of mmap and msync for NODEFS
196e459
to
3188415
Compare
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.
Great, thanks @petersalomonsen !
add implementation of mmap and msync for NODEFS