zargony / fuse-rs

Rust library for filesystems in userspace (FUSE)
MIT License
1.05k stars 131 forks source link

Suggestion: make missing reply a more critical error #123

Open wwylele opened 5 years ago

wwylele commented 5 years ago

Currently if a ReplyRaw drops without a reply, it sends a EIO and logs a warning https://github.com/zargony/rust-fuse/blob/73b841a544e33f58d1034df247457ce40208ec83/src/reply.rs#L189-L194

This however is not good for debugging errors like forgetting to put a reply in the code. The usage of log is not documented, while one must be aware of it and link a logger library in the executable (and potentially adjust the log level) to see the log. This makes the default behaviour effectively silent EIO reply, which can be mixed with other intended EIO from the executable. IMO the library should just panic on missing reply, as one should always explicitly return EIO if it is intended, and missing reply is almost definitely an mistake.

If you insist on keeping this a warning, please document the usage of log to help debugging.

wwylele commented 5 years ago

btw when will be the next release? I want to ditch the ugly time stuff in my code :smile:

zargony commented 5 years ago

You're right that the usage of the log crate should be documented better. The examples show how env_logger is initialized to get log output, but I agree that this can be done better and should be mentioned in the README.

I personally don't like panics in libraries. While you're right that a dropped reply is probably an implementation error, a library should never panic but return errors/warnings instead. Actually it would be best to enforce the implementation to always return a reply (e.g. using the return value), which is how it probably will look like anyway once I find time to finish async support.