hannobraun / inotify-rs

Idiomatic inotify wrapper for the Rust programming language
ISC License
261 stars 65 forks source link

Consider increasing the buffer size for the stream example #186

Closed epompeii closed 2 years ago

epompeii commented 3 years ago

Currently, the buffer size of the stream example is set to let mut buffer = [0; 32];. When I tried to run the example, I hit a rather cryptic OS error, Invalid Argument. This was easily fixed by increasing the buffer size.

The unit test uses let mut buffer = [0; 1024];. Maybe the example should use 1024 as well?

It is unlikely that tempfile would produce a path in either case that would cause the inotify message to exceed 1024. I've come up with a function though that given a filepath it will calculate the maximum inotify message size one could expect.

Please, let me know if either of these would be worth opening an MR for.

fn get_buffer_size(path: &Path) -> usize {
    // Get the length of the parent path if the filepath is not the root directory.
    // Because we canonicalized the filepath above, we do not need to worry about
    // prefixes.
    let name_max = if let Some(parent_path) = path.parent() {
        parent_path.as_os_str().len()
    } else {
        0
    }
    // Add one to account for a `/`, either in between the parent path and a filename
    // or for the root directory.
    + 1
    // Add the maximum number of chars in a filename, 255
    // https://github.com/torvalds/linux/blob/master/include/uapi/linux/limits.h
    // Unfortunetely, we can't just do the same with max path length itself.
    // https://eklitzke.org/path-max-is-tricky
    + 255;

    // inotify event max size formula:
    // sizeof(struct inotify_event) + NAME_MAX + 1
    // https://man7.org/linux/man-pages/man7/inotify.7.html
    std::mem::size_of::<inotify_sys::inotify_event>() + name_max + 1
}
hannobraun commented 3 years ago

Thanks for bringing this up, @epompeii. I have no idea why the buffer size in that example is so small. Increasing to 1024 is reasonable.

Your buffer size function looks good. Maybe we can include it as a utility function in the library, or did you have something else in mind?

epompeii commented 3 years ago

Sounds good! I've opened up PR https://github.com/hannobraun/inotify-rs/pull/187 that includes both the buffer size change and two utility functions.