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

In case of early ejection of SD card, allow "closing" RawFile handle anyway #122

Closed
peterkrull opened this issue Mar 25, 2024 · 3 comments
Closed

Comments

@peterkrull
Copy link
Contributor

peterkrull commented Mar 25, 2024

It seems that there is no nice way to get rid of a RawFile handle if for example the SD card is ejected, since calls to crate::VolumeManager::close_file will result in a returned error before the file id is removed from the open_files list.

In my testing, I have implemented the following:

if volume_mgr.write(file, data.as_slice()).is_err() {
    volume_mgr.destroy_file(file).unwrap();
    volume_mgr.close_dir(dir).unwrap();
    volume_mgr.close_volume(volume).unwrap();

    continue 'setup; // Loop back and retry connecting to card
}

where

/// Destroy a file for the given RawFile, without updating its entry.
/// This can be useful if the SD card is suddenly ejected, and one must
/// notify the VolumeManager that we are dropping the handle.
pub fn destroy_file(&mut self, file: RawFile) -> Result<(), Error<D::Error>> {
    let file_idx = self.get_file_by_id(file)?;
    self.open_files.swap_remove(file_idx);
    Ok(())
}

Which works nicely. Another option would be to modify the close_file method to first get the index, remove from the list, and then try to update the entry by flusing (as implemented in #121):

/// Close a file with the given full path.
pub fn close_file(&mut self, file: RawFile) -> Result<(), Error<D::Error>> {
    let file_idx = self.get_file_by_id(file)?;
    self.open_files.swap_remove(file_idx);
    self.flush(file)?;
    Ok(())
}
@thejpster
Copy link
Member

I think close should always close a file, regardless of whether it was able to flush it.

@peterkrull
Copy link
Contributor Author

I think the simplest solution is to simply not short-circuit the file-flush, then remove the file index, then return the result of the file-flush

@peterkrull
Copy link
Contributor Author

Closing in favor of #123

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