Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

修正了FAT32判断逻辑,解决了文件系统为FAT12/16时系统无法正常启动的问题。 #211

Merged
merged 3 commits into from
Mar 27, 2023

Conversation

WaferJay
Copy link
Contributor

Crash

return self.total_sectors_16 == 0;

此处条件判断并不能有效区分FAT32和FAT12/16,导致了如果文件系统是FAT12/16的话, validate() 中的判断会走到 kerror()

Fix

  • 我所知的使用广泛的FAT类型的判断方法是根据簇的数量确定的。所以这里改成通过簇数确定FAT32还是FAT12/16。
  • 另外将 validate() 往后移了,等到所有字段加载好了、FAT类型也判断好后再检查FAT32的BPB是否正确。

Reference

@fslongjin
Copy link
Member

fslongjin commented Mar 26, 2023

  • 我所知的使用广泛的FAT类型的判断方法是根据簇的数量确定的。所以这里改成通过簇数确定FAT32还是FAT12/16。

是的,确实是根据簇的数量来判断,在这里,原本的代码是正确的。
https://github.com/DragonOS-Community/DragonOS/pull/211/files#diff-1baf50937f9cbc77bf309131bfb38475dd3b997359c11c86a399c7fe0ea9d142L261

FAT specification里面有提到:

In the following example, when it says <, it does not mean <=. Note also that the numbers are correct. The first number for FAT12 is 4085; the second number for FAT16 is 65525. These numbers and the ‘<’ signs are not wrong.

If(CountofClusters < 4085) {
/* Volume is FAT12 */
} else if(CountofClusters < 65525) {
    /* Volume is FAT16 */
} else {
    /* Volume is FAT32 */
}

This is the one and only way that FAT type is determined. There is no such thing as a FAT12 volume that has more than 4084 clusters. There is no such thing as a FAT16 volume that has less than 4085 clusters or more than 65,524 clusters. There is no such thing as a FAT32 volume that has less than 65,525 clusters. If you try to make a FAT volume that violates this rule, Microsoft operating systems will not handle them correctly because they will think the volume has a different type of FAT than what you think it does.
  • 另外将 validate() 往后移了,等到所有字段加载好了、FAT类型也判断好后再检查FAT32的BPB是否正确。

你提到的这个是正确的,我想,这应该就是发生错误的原因:原本的代码在不知道当前是否为FAT32的情况下,就对bpb32做校验。
我做了一下测试,只保留你在这里的更改(就是,把validate往后移动的操作)就能解决问题。我想,其他地方的修改应该是不需要的。
https://github.com/DragonOS-Community/DragonOS/pull/211/files#diff-1baf50937f9cbc77bf309131bfb38475dd3b997359c11c86a399c7fe0ea9d142R270

@WaferJay
Copy link
Contributor Author

WaferJay commented Mar 26, 2023

@fslongjin 你的考虑很对。前面之所以提FAT类型判断是因为开头引用的那个判断函数名是 is_fat32()。我觉得它有这几点问题,所以除了移动 validate() 还去掉了 is_fat32() 相关的代码。

  • 首先它功能上并不符合它的名字,它的判断条件是错误的,FAT16也可以满足条件。
  • 另外移动 validate() 后,即便它的职责符合函数名的意思,调用它时显然已经满足is fat32,它也就成了无效的代码。

代码除了实现功能,还要考虑可维护性,尽量避免代码异味。

@fslongjin
Copy link
Member

  • 它的判断条件是错误的,FAT16也可以满足条件。

我刚刚查了一下文档,确实像你说的. 这个is_fat32()的判断条件是一个必要不充分条件。
validate()的起始部分也对其他的问题做了校验。这些校验同样适用于FAT12/16,如果只对FAT32调用validate(),相当于跳过了对FAT12/16的校验。因此,我觉得更改is_fat32()会更合适.在这个`is_fat32()里面增加对fattype的校验。

…BlockFAT32.validate() and BiosParameterBlockLegacy.validate()
@WaferJay
Copy link
Contributor Author

WaferJay commented Mar 26, 2023

validate() 职责不够单一,is_fat32 参合的条件判断代码显得混乱。已经将 validate() 拆分了。BiosParameterBlock.validate() 仅对公共字段做校验,并把FAT32的校验委派给 BiosParameterBlockFAT32.validate()、把FAT12/16的校验委派BiosParameterBlockLegacy.validate()

因为 validate() 调用已经放置在最后,所以 FAT 类型已经确定,不需要 is_fat32 这个外部不调用的字段了。

@fslongjin fslongjin merged commit 2286eda into DragonOS-Community:master Mar 27, 2023
fengjixuchui added a commit to fengjixuchui/DragonOS that referenced this pull request Mar 27, 2023
修正了FAT32判断逻辑,解决了文件系统为FAT12/16时系统无法正常启动的问题。 (DragonOS-Community#211)
MorkCarpenter pushed a commit to MorkCarpenter/DragonOS-fork-Mork that referenced this pull request Mar 27, 2023
* fix(fat): fix determination of fat type casue crash if fs is fat12/16

* refactor(fat): split BiosParameterBlock.validate() into BiosParameterBlockFAT32.validate() and BiosParameterBlockLegacy.validate()

* 调整“最大允许的簇号”的常量放置的位置。

---------

Co-authored-by: longjin <[email protected]>
MorkCarpenter pushed a commit to MorkCarpenter/DragonOS-fork-Mork that referenced this pull request Mar 28, 2023
* fix(fat): fix determination of fat type casue crash if fs is fat12/16

* refactor(fat): split BiosParameterBlock.validate() into BiosParameterBlockFAT32.validate() and BiosParameterBlockLegacy.validate()

* 调整“最大允许的簇号”的常量放置的位置。

---------

Co-authored-by: longjin <[email protected]>
fslongjin added a commit that referenced this pull request Apr 28, 2023
* 新增VFS文档,以及修改文档配置 (#209)

* 1.新增vfs设计文档
2.修改文档版权标志为"2022-2023, DragonOS Community"
3.修改电脑版文档页面的宽度为90%

* layout.html末尾加空行

* 修正了FAT32判断逻辑,解决了文件系统为FAT12/16时系统无法正常启动的问题。 (#211)

* fix(fat): fix determination of fat type casue crash if fs is fat12/16

* refactor(fat): split BiosParameterBlock.validate() into BiosParameterBlockFAT32.validate() and BiosParameterBlockLegacy.validate()

* 调整“最大允许的簇号”的常量放置的位置。


* 增加x87FPU支持 (#212)

* remove `ret_from_syscall`
*修复ps2键盘驱动程序inode在进程fork的时候导致死锁的问题.
*更新: VFS每次拷贝文件描述符的时候,都会去调用inode的open函数



* 部分函数从返回值为Result<<>,i32>修改为Result<<>,SystemError> (#210)

* 将Result<<>,i32>替换为Result<<>,SystemError>
* bugfix: 显示双缓冲区初始化的时候,连续注册了两次Video Softirq的问题。



* 第一套键盘扫描码的状态机 (#216)

第一套键盘扫描码的状态机

* 将TTY与stdio进行连接,实现基本的stdio功能 (#217)

* 将stdio与tty接上

* Patch keyboard capslock alt (#219)

* keyboard-alt-capslock

* 解决键盘输入'%'字符的时候无法回显的bug



* Add dup,dup2 (#224)

* dup,dup2

* fix: sys_dup2语义与posix不一致的问题


* 添加了qemu使用VNC作为图像输出的选项 (#222)

* 添加了qemu使用VNC作为图像输出的选项

* 设置vnc端口为5900


* 软中断&定时器重构 (#223)

* 软中断&定时器重构


* 修改timer的clock()

* 删除debug信息



* V0.1.6发行日志&更新构建系统文档 (#225)

1.更新构建系统文档
2.V0.1.6发行日志

* Patch add 0.1.6 changelog (#226)

* 修正标题错误

* 修复显示刷新线程的空指针问题 (#228)

* 新增设备驱动模型,为设备和驱动提供高层视图 (#227)

* 添加base mod

* 添加设备驱动模型相关文件

* 删除单独的mod文件,使用mod.rs,修改一些格式上的问题

* 移动驱动错误类型到该文件

* 修改一些格式上的问题

* 修改CFSqueue从Vec变成红黑树 (#229)

使用了由tickbh编写的rbtree: https://github.com/tickbh/rbtree-rs/blob/master/src/lib.rs


* new: lazy_init (#230)

* Patch add lazy init (#236)

* 修正并发安全问题

* pci重构+pcie支持 (#235)

* pci重构+pcie支持

* pci重构测试完成

* 修正makefile的问题

* 小修改

* 修改函数名字

* 增加对dhcpv4的支持(tcp、udp socket已写好,但由于缺少epoll机制,尚未完整测试) (#237)

* 为virtio网卡完成smoltcp的phy层配置

* raw socket

* 初步写完udp和tcp socket

* 能够正常通过dhcp获取ipv4地址(具有全局iface btree)



* 调整brk系统调用,使得参数、返回值与Linux一致 (#238)

* 新增用于测试relibc的app

* 为适配relibc,修改do_execve中关于用户栈的内容的设置

* 调整brk系统调用,使得参数、返回值与Linux一致

* 修改errno,使其与relibc的保持一致 (#234)

修改errno,使其与relibc的保持一致

* 修复ecam无法获取MCFG table的问题 (#241)

* 修复Issue#220;vnc的端口号恢复5900 (#243)


* 修复Issue#220

* qemu-vnc端口号恢复为5900

* new: DowncastArc and its docs (#244)

* 增加定时器和软中断文档,修改了softirq面向c的接口 (#245)

* 增加定时器和软中断文档

* 修改softirq对c的接口和文档

* 修改文档格式

* 新增网络socket的系统调用接口 (#247)

1.修复spinlock忘记恢复rflags的问题
2.WaitQueue增加wakeup_all的功能
3.完善tcp,udp,raw socket
4.把PollStatus结构体改为使用bitflags
5.新增iovec结构体
6.完成网络的系统调用
7.在bootstrap里面添加dnsmasq bridge-utils iptables

* 新增SysFS (#250)

* 添加sysfs

* 注册sysfs

* 添加sysfs相关

* 添加rust-anlyzer辅助配置

* 将设备与sysfs相关联

* 添加单独的文件管理sysfs下的文件夹

* 解决使用zsh在构建DragonOS时,无法直接使用一键初始化脚本进行安装的问题  (#252)

* 匿名管道重构&增加IrqArch trait以及IrqFlags及其守卫 (#253)

* 实现匿名管道

* 增加IrqArch trait以及IrqFlags及其守卫


* 根据sysfs完善设备驱动模型 & 添加sysfs官方文档 (#254)

* 根据sysfs完善设备驱动模型

* 添加sysfs官方文档

* doc: V0.1.7发行日志 (#255)
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