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

Add direct tests for all shims #3179

Open
RalfJung opened this issue Nov 20, 2023 · 3 comments
Open

Add direct tests for all shims #3179

RalfJung opened this issue Nov 20, 2023 · 3 comments
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement E-good-first-issue A good way to start contributing, mentoring is available

Comments

@RalfJung
Copy link
Member

RalfJung commented Nov 20, 2023

For example: "lseek" on Linux is currently only tested via the libstd functions, but not via direct libc calls.

And really we should ideally have direct tests for all of our shims to make sure they don't start to bitrot when the standard library changes. So the first step here would be to figure out which of our shims still need testing. Just assembling the TODO list is already quite valuable!

@RalfJung RalfJung added the E-good-first-issue A good way to start contributing, mentoring is available label Nov 20, 2023
@irfanzainudin
Copy link

irfanzainudin commented Nov 28, 2023

Hi there! I'm interested in contributing and thought I'd start with this issue which sounds doable. I'm studying software engineering at uni and I had some experience with C and currently going deeper into Rust after taking a course at uni.

From what I understand so far for this issue, I wrote some code and this is what I have so far and I was wondering if I'm in the right direction?

tests/pass-dep/shims/libc-fs.rs

fn main() {
    // ...
    test_ftruncate();
}

fn test_ftruncate() {
    // Create a file with some content
    // Taken from the test functions above
    let path = prepare_with_content("test_ftruncate.txt", &[]);

    let mut name = path.into_os_string();
    name.push("\0");
    let name_ptr = name.as_bytes().as_ptr().cast::<libc::c_char>();
    let fd = unsafe { libc::open(name_ptr, libc::O_RDONLY, 42) };

    // We use i64 here because the second argument for ftruncate()
    // uses off_t which is a signed integer types and on my machine,
    // off_t is synonymous with long long
    // References:
    // - https://doc.rust-lang.org/std/os/raw/type.c_longlong.html
    // - https://locka99.gitbooks.io/a-guide-to-porting-c-to-rust/content/features_of_rust/types.html
    // - https://stackoverflow.com/questions/9073667/where-to-find-the-complete-definition-of-off-t-type
    let l: i64 = 4;
    let length = l.cast::<libc::c_longlong>(); // not too sure about this line
    let res = unsafe { libc::ftruncate(fd, length) };
    if res == 0 {
        // call succeeds
    } else {
        // call fails
    }
}

@RalfJung
Copy link
Member Author

Right, basically these tests should be fairly similar to this one but directly using the libc APIs rather than calling Rust's std::fs functions.

You can use the CString type to generate C-compatible strings, no need to manually push \0.

@RalfJung
Copy link
Member Author

Support for extra modes in open (as implemented in #3240) should also be covered by such a test.

@RalfJung RalfJung changed the title Add direct tests for libc FS functions like "ftruncate" Add direct tests for libc FS functions Feb 12, 2024
@RalfJung RalfJung changed the title Add direct tests for libc FS functions Add direct tests for all shims Mar 4, 2024
bors added a commit that referenced this issue Mar 24, 2024
Add libc direct test for shims and update CONTRIBUTING.md

Add more ``libc`` direct test for shims as mentioned in #3179.

Changes:
- Added ``libc`` direct tests for ``rename`` and ``ftruncate64``
- Added the command for running ``pass-dep`` test in ``CONTRIBUTING.md``
@RalfJung RalfJung added the A-shims Area: This affects the external function shims label Apr 6, 2024
@RalfJung RalfJung added the C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-shims Area: This affects the external function shims C-enhancement Category: a PR with an enhancement or an issue tracking an accepted enhancement E-good-first-issue A good way to start contributing, mentoring is available
Projects
None yet
Development

No branches or pull requests

2 participants