SR-8999: NSData contentsOf expects st_size to be the size of the file. #1732
Conversation
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
- Modify FileHandle._readDataOfLength(untilEOF:options:) to ignore stat.st_size and just use the result of read() to determine whether to keep reading. - Also convert syscall error handling to throw instead of calling fatalError(). - NSData.readBytesFromFileWithExtendedAttributes(options:) now uses FileHandle._readDataOfLength(untilEOF:options:) to read from a file rather than duplicating the file reading logic. - (Linux) Add a test to read a file in /sys which reports stat.st_size > size of the file's contents.
|
@swift-ci test |
- readDataToEndOfFile(), readData(ofLength length: Int) and .availableData should call fataError() if there are IO errors, not return an empty Data().
|
Ive restored the correct behaviour. |
|
@swift-ci test |
1 similar comment
|
@swift-ci test |
millenomi
approved these changes
Oct 23, 2018
|
|
||
| let readBlockSize: Int | ||
| if statbuf.st_mode & S_IFMT == S_IFREG { | ||
| // TODO: Should files over a certain size always be mmap()'d? |
There was a problem hiding this comment.
Immediate answer: no, unless if options contains .alwaysMapped or .mappedIfSafe.
(I don't see .mappedIfSafe handled here at all? Are you just assuming mapping is always unsafe? This is allowed by the contract but we should have a follow-up.)
There was a problem hiding this comment.
.mappedIfSafe wasn't handled in the previous code so I haven't added it but I can do a separate PR for it.
|
@itaiferber @phausler If you want to chip in today, or I'll merge. |
|
Seems reasonable to me |
phausler
approved these changes
Oct 23, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Modify FileHandle._readDataOfLength(untilEOF:options:) to ignore
stat.st_size and just use the result of read() to determine whether
to keep reading.
Also convert syscall error handling to throw instead of calling
fatalError().
NSData.readBytesFromFileWithExtendedAttributes(options:) now uses
FileHandle._readDataOfLength(untilEOF:options:) to read from a file
rather than duplicating the file reading logic.
(Linux) Add a test to read a file in /sys which reports
stat.st_size > size of the file's contents.