Skip to content

Read font info files incrementally - #21

Merged
dclamage merged 1 commit into
PathOfBuildingCommunity:masterfrom
zao:fix-font-reading
May 3, 2021
Merged

Read font info files incrementally#21
dclamage merged 1 commit into
PathOfBuildingCommunity:masterfrom
zao:fix-font-reading

Conversation

@zao

@zao zao commented May 3, 2021

Copy link
Copy Markdown
Contributor

The previous text-mode reading had a problem where the extractable character count of a file opened in non-binary (text) mode is subject to the actual contents of the file due to newline translation and EOF characters, and can only be determined by actually scanning over the content.

GetLen yields the binary size of the file and thus tends to overallocate the buffer, leading to the actual read attempting to read more data than can actually be extracted. This puts the code at the mercy of what garbage the target string contains from the allocator and how much data could actually be read.

The previous text-mode reading had a problem where the extractable character count of a file opened in non-binary (text) mode is subject to the actual contents of the file due to newline translation and EOF characters, and can only be determined by actually scanning over the content.

GetLen yields the binary size of the file and thus tends to overallocate the buffer, leading to the actual read attempting to read more data than can actually be extracted. This puts the code at the mercy of what garbage the target string contains from the allocator and how much data could actually be read.

@dclamage dclamage left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks!

@dclamage
dclamage merged commit 4b386cc into PathOfBuildingCommunity:master May 3, 2021
@zao
zao deleted the fix-font-reading branch October 16, 2023 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants