Replace POSIX getline with std::getline#184
Open
etorth wants to merge 4 commits into
Open
Conversation
Use C++ standard stream-based line reading in tests and utilities so the code does not depend on POSIX getline availability on MinGW UCRT64. Refs libpinyin#183. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The libpinyin model tables are UTF-8 text files with ASCII field separators. Parsing them with fscanf("%s") is not safe after gen_binary_files calls setlocale(LC_ALL, ""), because the C runtime decides what counts as whitespace according to the active locale.
On MinGW/UCRT this can split a UTF-8 Chinese character in the middle. One failing case is the valid source row:
lao 唠 16782070 477
where 唠 is encoded as E5 94 A0. In the MinGW build log, ChewingLargeTable2::load_text reported the phrase as truncated bytes:
ChewingLargeTable2::load_text:lao E5 94 16782070 477
That shows UCRT treated the 0xA0 byte inside the UTF-8 sequence as whitespace. The phrase then becomes invalid UTF-8, so g_utf8_strlen() and the parsed pinyin key count no longer match. The result is not only noisy diagnostics: the generated indexes can miss or corrupt entries. Native Linux does not show this because glibc's UTF-8 locale does not classify those high UTF-8 bytes as whitespace.
Use line-based table parsing so field boundaries are determined by the model file format rather than CRT locale behavior. This also avoids the old %ld conversion into size_t, which is incorrect on Windows LLP64.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MemoryChunk::load("gb_char.bin") fails with "load gb_char.bin failed!" after gen_binary_files writes gb_char.bin and import_interpolation reads it back.
On MinGW, open() defaults to text mode. MemoryChunk::save() wrote binary table data without O_BINARY, so LF bytes in the payload were expanded to CRLF. MemoryChunk::load() then saw a payload whose actual size/checksum no longer matched the stored header and rejected it as corrupted.
The generated file showed file_size=2979848, header_len=2972089, actual_data_len=2979840, difference=7751, and crlf_pairs_in_payload=7751. The exact match between the size difference and CRLF pairs proves text-mode newline expansion caused the corruption.
Use O_BINARY for MemoryChunk load and save paths, defining it as 0 when unavailable. This fixes MinGW while leaving Linux and macOS behavior unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
Could you make the second and third commit message shorter? I understand the issues now, maybe you can put the details of the commit messages into this pull request or the previous issue. For the O_BINARY macro, how about to use the following code snippet? |
Author
Let me make sure the main project build passes before focusing on this libpinyin PR. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Use C++ standard stream-based line reading in tests and utilities so the code does not depend on POSIX getline availability on MinGW UCRT64.
Refs #183.