Skip to content

Comments

fix the file scan error related to CRLF format#6472

Merged
nihui merged 3 commits intoTencent:masterfrom
chennevwin:fix-android-scan-crlf
Dec 31, 2025
Merged

fix the file scan error related to CRLF format#6472
nihui merged 3 commits intoTencent:masterfrom
chennevwin:fix-android-scan-crlf

Conversation

@chennevwin
Copy link
Contributor

目的

修复若使用CRLF格式换行的配置文件,调用int DataReaderFromAndroidAsset::scan(const char* format, void* p)函数导致解析异常的问题

问题说明

当采用Windows开发Android项目,由于git core.autocrlf 功能,会将配置文件换行转为CRLF风格的文件,导致执行DataReaderFromAndroidAsset::scan解析数据出现问题

问题举例

假如输入数据是
"123\r\n456"

第一次调用
dr.scan("%d", &a);

走else处理

newline_pos = (const char*)memchr((const char*)d->mem, '\n', remain_length);

size_t line_length = newline_pos ? newline_pos - (const char*)d->mem : (size_t)remain_length;
line = std::string((const char*)d->mem, line_length);

得 line = "123\r"

int nscan = sscanf(line.c_str(), format_with_n, p, &nconsumed);

得到
nconsumed = 3
p = 123

d->mem += nconsumed;

此刻 d->mem 指向的是 '\r' 后续数据就是 "\r\n456"

当第二次调用
dr.scan("%d", &a);
由于'\r'才是第一个字符,还是走的else

newline_pos = (const char*)memchr((const char*)d->mem, '\n', remain_length);

size_t line_length = newline_pos ? newline_pos - (const char*)d->mem : (size_t)remain_length;
line = std::string((const char*)d->mem, line_length);

得 line = "\r"

到这里执行解析就不正确了,这并不是我们预期的,正确处理应该是类似\n处理,跳过\r\n读后续的数据

@github-actions github-actions bot added the core label Dec 30, 2025
@tencent-adm
Copy link
Member

tencent-adm commented Dec 30, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a parsing issue in DataReaderFromAndroidAsset::scan() when configuration files use CRLF (Windows-style) line endings. When scanning data with CRLF line endings, the function would fail to properly skip over the \r\n sequence between lines, causing the parser to point to the leftover \r character and fail on subsequent scans.

Key Changes:

  • Added CRLF detection and handling logic to skip \r\n sequences at the beginning of a line, mirroring the existing logic for LF-only line endings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

// however, it is fine to create "\nXYZ 123 abc" as sscanf will skip the leading newline silently
newline_pos = (const char*)memchr((const char*)d->mem + 1, '\n', remain_length - 1);
}
else if(remain_length > 2 && ((const char*)d->mem)[0] == '\r' && ((const char*)d->mem)[1] == '\n'){
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

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

The condition 'remain_length > 2' should be 'remain_length >= 2' since you only need to access indices 0 and 1, which requires at least 2 bytes, not more than 2 bytes.

Suggested change
else if(remain_length > 2 && ((const char*)d->mem)[0] == '\r' && ((const char*)d->mem)[1] == '\n'){
else if (remain_length >= 2 && ((const char*)d->mem)[0] == '\r' && ((const char*)d->mem)[1] == '\n')
{

Copilot uses AI. Check for mistakes.
@codecov-commenter
Copy link

codecov-commenter commented Dec 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.90%. Comparing base (39003ec) to head (b775fa2).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6472   +/-   ##
=======================================
  Coverage   95.90%   95.90%           
=======================================
  Files         844      844           
  Lines      265298   265299    +1     
=======================================
+ Hits       254431   254432    +1     
  Misses      10867    10867           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

chennevwin and others added 2 commits December 30, 2025 22:52
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@nihui nihui closed this Dec 30, 2025
@nihui nihui reopened this Dec 30, 2025
@nihui nihui merged commit faea660 into Tencent:master Dec 31, 2025
107 of 110 checks passed
@nihui
Copy link
Member

nihui commented Dec 31, 2025

Thanks for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants