Support directory deletion #210
Conversation
estebank
force-pushed
the
esteban/directory-deletion
branch
from
0eebda1 to
ae73f2f
Compare
estebank
force-pushed
the
esteban/directory-deletion
branch
from
ae73f2f to
cf1eca0
Compare
| .iter() | ||
| .find(|dir_info| { | ||
| // Subdirectory is already open. | ||
| dir_info.cluster == dir_entry.cluster |
There was a problem hiding this comment.
I think that if the directory you are deleting is already open, you shouldn't be able to delete it because it would leave a dangling entry.
| let dir_idx = data.get_dir_by_id(directory)?; | ||
| let dir_info = &data.open_dirs[dir_idx]; | ||
| let volume_idx = data.get_volume_by_id(dir_info.raw_volume)?; | ||
| let parent_dir_info = data.open_dirs[dir_idx].clone(); |
There was a problem hiding this comment.
I don't recall needing to clone an entry for an open directory before (unless I'm forgetting about it). What's the reason for the clone as opposed to borrowing the entry, or referring to it by index? This is probably fine, it just gave me pause as I read it.
There was a problem hiding this comment.
This is only needed if we want to allow deletion of currently open directories, because I'm creating a new DirectoryInfo in a block and making it a borrow would require using some of the lifetime extension tricks, while the struct looked tiny enough that cloning seemed cleaner to do.
|
Rebasing after #211 is merged should fix CI. |
2 participants
Fix #186.
The changes to the
Errorenum would be a breaking change. The new variant can either be only added, leaving behind a variant that will never be constructed or replaced.