Skip to content

Remove thread-blocking call to libc::stat in Path::stat #9958

Description

@alexcrichton

This should instead use the rt::io::file interface which does not block the thread. Additionally, this should remove the default_stat functions in the path/mod.rs file along with related definitions.

This is a pretty good first project for anyone wanting to sink their teeth into some library work! It's a bit of refactoring and should result in a nice large negative diffstat!

Activity

  1. hatahet commented on Oct 20, 2013

    @hatahet
    Contributor

    What about calls to lstat() in path/posix.rs?

  2. hatahet commented on Oct 20, 2013

    @hatahet
    Contributor

    It seems that there is no 1-to-1 mapping of libc::stat to rt::io::FileStat. Particularly, the latter is missing fields, such as mode, and OS-specific attributes, such as birthtime on MacOS and FreeBSD. I suppose that FileStat will have to have those fields added to it.

  3. alexcrichton commented on Oct 20, 2013

    @alexcrichton
    MemberAuthor

    The lstat function should not exist in that current form, and it's fine to drop fields for now. A rust Path should work the same on all platforms, not create different structs with different fields (very easy to miscompile on other platforms).

  4. hatahet commented on Oct 21, 2013

    @hatahet
    Contributor

    When I changed the implementation of Path::stat() in posix.rs to call rt::io::file::stat() and then make check, I hit a compiler error:

    task '<unnamed>' failed at 'Unhandled condition: io_error: rt::io::IoError{kind: OtherIoError, desc: "no such file or directory", detail: None}', /Users/.../dev/local/github/rust/src/libstd/condition.rs:131
    error: internal compiler error: unexpected failure
    note: the compiler hit an unexpected failure path. this is a bug
    note: try running with RUST_LOG=rustc=1 to get further details and report the results to github.com/mozilla/rust/issues
    task '<unnamed>' failed at 'explicit failure', /Users/.../dev/local/github/rust/src/librustc/rustc.rs:395
    

    I do have an export RUST_LOG=rustc=1, but nothing extra seems to be printed. Should I file a separate issue for this?

  5. alexcrichton commented on Oct 21, 2013

    @alexcrichton
    MemberAuthor

    The calls to rt::io::file::stat will raise on the io_error condition when they encounter an error. In this case it looks like the error was that a file was not found. The return value of stat will be None, but you'll need to explicitly catch the error as well for now (and ignore it or continue to propagate it).

  6. hatahet commented on Oct 22, 2013

    @hatahet
    Contributor

    If it is fine to drop fields for now, I suppose we should remove tests that reference such fields. E.g. in libstd/run.rs:587 we have assert_eq!(parent_stat.st_dev, child_stat.st_dev); for two tests: test_keep_current_working_dir() and test_change_working_directory

  7. hatahet commented on Oct 22, 2013

    @hatahet
    Contributor

    Both the fields in question exist in uv_stat_t, I suppose I will just add them to FileStat then in that case.

  8. alexcrichton commented on Oct 22, 2013

    @alexcrichton
    MemberAuthor

    Feel free to modify FileStat as much as you need, especially if there's information inside of uv_stat_t on all platforms, then feel free to add it in!

  9. added 2 commits that reference this issue on Dec 17, 2022
    7a0f0c0
    56d5657
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions