Skip to content

Fix: wrong comment in filesystem atomic operation section#46

Merged
Charliechen114514 merged 1 commit into
Awesome-Embedded-Learning-Studio:mainfrom
YukunJ:fix-fs-comment
Jun 11, 2026
Merged

Fix: wrong comment in filesystem atomic operation section#46
Charliechen114514 merged 1 commit into
Awesome-Embedded-Learning-Studio:mainfrom
YukunJ:fix-fs-comment

Conversation

@YukunJ

@YukunJ YukunJ commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Brief: fs::copy_file does not guarantee atomic file operations. I felt a bit confused by the comments here in the code sample. It seems like a typo because the paragraph above already stated clearly std::copy_file has no atomicity guarantee.

@Charliechen114514

@Charliechen114514

Copy link
Copy Markdown
Member

我仔细看看,发现的确,我在原文那里typo了:原代码注释 // 安全的文件复制(原子性保证) 与紧邻其上的正文「copy_file 不提供原子性保证」直接矛盾,属实质性笔误,并非单纯措辞问题。改成 // 不安全的文件复制(无原子性保证) 后语义前后一致。

非常感谢这两条增补注释技术:copy_file 在 POSIX 实现上对应 sendfile / read+write 循环直接覆盖目标 inode,确实不具备原子性,两条失败场景(被逐步覆盖、断电导致损坏)描述无误。

感谢捉虫!

I took a closer look and realized, indeed, I made a typo in the original text: the original code comment // Safe file copy (atomic guarantee) directly contradicts the nearby text saying 'copy_file does not provide atomic guarantees.' It's genuinely a substantive typo, not just a wording issue. Changing it to // Unsafe file copy (no atomic guarantee) makes the meaning consistent throughout.

Really appreciate these two supplementary technical notes: in the POSIX implementation, copy_file corresponds to sendfile / read-write loops that directly overwrite the target inode, so it truly isn't atomic. The two failure scenarios (being progressively overwritten, damage due to power loss) are accurately described.

Thanks for correctness!

@Charliechen114514 Charliechen114514 merged commit c1d5dd1 into Awesome-Embedded-Learning-Studio:main Jun 11, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants