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

对simple-fat32设计的一些建议 #4

Open
jklincn opened this issue May 2, 2022 · 5 comments
Open

对simple-fat32设计的一些建议 #4

jklincn opened this issue May 2, 2022 · 5 comments

Comments

@jklincn
Copy link

jklincn commented May 2, 2022

fat32_manager即fat32管理器模块中,对文件名的一些操作,比如拆分文件名和后缀、拆分长文件名、短文件名格式化、由长文件名生成短文件名等,我觉得这些算作“工具函数”,没有必要非要和 FAT32Manager 捆绑在一起(即作为它的implement),以至于每次使用这些工具时都需要获取对管理器的读锁。

我在手动把这些函数取出 impl 块后,修改对应的报错非常快,基本只要把函数前面对结构体的引用删去即可,这一实践也说明了这些函数与FAT32Manager的关联程度并不大

@jklincn
Copy link
Author

jklincn commented May 2, 2022

其他问题:

  1. vfs.rs 中,VFile的方法定义中有一个 clear_cache,首先从代码上讲,这个方法没有被调用过,可以删去;其次从逻辑上来讲,VFile是虚拟文件,应该不能对文件系统块缓存进行操作。
  2. layout.rs 中,长文件名目录项的 clear 方法似乎没有完成?它只有一行语句且被注释了,然而这个函数在 vfs 中 VFile 的 clear 中有被使用到

@SocialistDalao
Copy link
Collaborator

SocialistDalao commented May 3, 2022

fat32_manager即fat32管理器模块中,对文件名的一些操作,比如拆分文件名和后缀、拆分长文件名、短文件名格式化、由长文件名生成短文件名等,我觉得这些算作“工具函数”,没有必要非要和 FAT32Manager 捆绑在一起(即作为它的implement),以至于每次使用这些工具时都需要获取对管理器的读锁。

我在手动把这些函数取出 impl 块后,修改对应的报错非常快,基本只要把函数前面对结构体的引用删去即可,这一实践也说明了这些函数与FAT32Manager的关联程度并不大

Great suggestions. What you said is exactly right, and it should be done this way. I think the best way is to add FAT32-irrelevant codes to support FAT32 implementation.

其他问题:

  1. vfs.rs 中,VFile的方法定义中有一个 clear_cache,首先从代码上讲,这个方法没有被调用过,可以删去;其次从逻辑上来讲,VFile是虚拟文件,应该不能对文件系统块缓存进行操作。
  2. layout.rs 中,长文件名目录项的 clear 方法似乎没有完成?它只有一行语句且被注释了,然而这个函数在 vfs 中 VFile 的 clear 中有被使用到

UltraOS call clear_cache() from os/src/fs/inode.rs to let kernel control the disk writeback operation. Theoretically, only root_inode is allowed to call this. However, we don't limit other nodes in our implementation, which should be corrected in VFS.

Operation clear() for long directory name is actually useless as what you said. It is designed for future use. However, obviously it has no "future". Actually, the kernel will create a short name of an directory while creating a long one. We only need to modify information of directory of short name. clear() is used to clear file contents rather than deleting them.

I suggest that we don't close this issue for future discussion related to kernel design. And we can let others think more from our discussion.

@jklincn
Copy link
Author

jklincn commented May 3, 2022

vfs.rs 中的 find_long_name 方法

                        for i in 0..order as usize { // 存入长名目录项位置了,第一个在栈顶
                            let pos = self.get_pos(offset + i);
                            long_pos_vec.push(pos);
                        }

是否应该为

                        for i in 0..order as usize { // 存入长名目录项位置了,第一个在栈顶
                            let pos = self.get_pos(offset + i * DIRENT_SZ);
                            long_pos_vec.push(pos);
                        }

另外,这个函数分支和循环结束时的 offset += DIRENT_SZ; 应该能合并

@SocialistDalao
Copy link
Collaborator

It's a little close to detail. I can't remember whether it should work this way, but you can try it to prove yourself.

@jklincn
Copy link
Author

jklincn commented May 3, 2022

It's a little close to detail. I can't remember whether it should work this way, but you can try it to prove yourself.

simple-fat32 中对偏移量的设计应该都是以字节为单位?这里是搜索长文件名目录项时,最后生成VFile返回的过程,把所有长文件名目录项的位置进行保存,get_pos是获取文件中该偏移量的扇区号和偏移位置,因此这边偏移量的传入我觉得应该是最初的偏移量加上 i * DIRENT_SZ 以表示下一个长文件名目录项,而不是简单的 i

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

No branches or pull requests

2 participants