ids1024 / iso9660-rs

Rust library for reading iso9660 filesystems
Apache License 2.0
22 stars 5 forks source link

Add try_into_inner() for ISO9660 and FileRef #2

Open losynix opened 2 years ago

losynix commented 2 years ago

Hello !

It would be nice to get ownership of the inner reader back when we're done. Not sure this is the best approach, FileRef makes this tricky but it works for me.

ids1024 commented 2 years ago

Not sure this is the best approach, FileRef makes this tricky but it works for me.

Hm, yeah. A try_into_inner function seems a bit odd to have, but I can see how this could be useful.

I'm not entirely happy with how reference counting is used in the crate in general, but I'm not sure the most efficient and ergonomic Rust API for ISO9660 (and filesystems generally; though is simpler in that it's read only).

ids1024 commented 2 years ago

Read is implemented for &mut T where T: Read, so where lifetime bounds aren't inconvenient, you can use something like ISO9660::new(&mut file) without consuming the original reader.

losynix commented 2 years ago

I'm not sure what the best approach for a filesystem API would be either. Other crates I checked like rust-fatfs or ext4-rs take ownership of the inner reader whereas ntfs asks for the reader in every functions accessing the filesystem.

Anyway you're right I can probably get around this by giving a reference to ISO9660::new(), thank you.