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

Remove example that works around issue #120 #125

Closed
hannobraun opened this issue Jan 28, 2019 · 3 comments · Fixed by #130
Closed

Remove example that works around issue #120 #125

hannobraun opened this issue Jan 28, 2019 · 3 comments · Fixed by #130

Comments

@hannobraun
Copy link
Owner

It's no longer as useful, as #120 is fixed now. It should be removed, and the technique it shows should be integrated into the API documentation.

@Atul9
Copy link
Contributor

Atul9 commented Jun 20, 2019

@hannobraun I would like to take this up. I have opened up a PR for this - #130

@emarcotte
Copy link

It seems like (unless I'm completely misunderstanding) the workaround demonstrated in this example only really works if:
1- Your buffer size is <= 32
2- You never get events with > 32

For instance, this dummy program:

let stream = inotify3
    .event_stream([0; 32])
    .map_err(|e| println!("inotify error: {:?}", e))
    .for_each(move |event| {
        println!("E: {:?}", event);
        Ok(())
    });
tokio::run(stream); 

Gets

 inotify error: Os { code: 22, kind: InvalidInput, message: "Invalid argument" }

When a file name is too long.

Seems like a limitation on arrays (check https://doc.rust-lang.org/std/primitive.array.html notes on array sizes as they related to AsMut/AsRef).

The best work around I've found for this is to include the bytes module as suggested in the original pull request. This seems to work so far:

    let mut inotify = Inotify::init().unwrap();
    // TODO: http://man7.org/linux/man-pages/man7/inotify.7.html
    // sizeof(struct inotify_event) + NAME_MAX + 1
    let mut buf = BytesMut::with_capacity(4096);
    for _ in 0..4096 {
        buf.put(0u8);
    }

    inotify.add_watch("/home/emarcotte/Projects/fwatch", WatchMask::ALL_EVENTS)
        .expect("Couldn't add watch");
    println!("Running...");

    let stream = inotify
        .event_stream(buf)
        .map_err(|e| println!("inotify error: {:?}", e))
        .for_each(move |event| {
            println!("E: {:?}", event);
            Ok(())
        });

    tokio::run(stream);

I guess all of this is to say, it might still be worth having a pretty decent example floating around to show how this works with larger file names/tokio integration/etc.

It might be nice to figure out how to provide the max size of the inotify event in order to build reasonably sized buffers. Of course, I have no idea how to do that yet :)

@hannobraun
Copy link
Owner Author

Thanks for your comment, @emarcotte! I agree with everything you said.

It seems like (unless I'm completely misunderstanding) the workaround demonstrated in this example only really works if:
1- Your buffer size is <= 32
2- You never get events with > 32

Yes, that is to be expected. I'm hoping that will care of itself soon-ish, once support for const generics lands in the compiler, and is then used to implement those traits for any array size.

The best work around I've found for this is to include the bytes module as suggested in the original pull request. This seems to work so far:

Thanks for the example. Really neat! A Vec<u8> should work too, although I haven't tried it.

I guess all of this is to say, it might still be worth having a pretty decent example floating around to show how this works with larger file names/tokio integration/etc.

Totally agree, but I think the best place for such an examples (or better, multiple such examples) is in the API documentation of the event_stream method. I've opened a follow-up issue, where I've linked to your comment. Hopefully someone will pick that up.

It might be nice to figure out how to provide the max size of the inotify event in order to build reasonably sized buffers. Of course, I have no idea how to do that yet :)

Yes, it would be great if someone figured out what a reasonable buffer size is, and added that information to the API documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants