samuela / rustybox

A free-range, non-GMO fork of busybox in 100% Rust 🦀
811 stars 33 forks source link

Cleanup fcntl imports #15

Closed Akeboshiwind closed 4 years ago

Akeboshiwind commented 4 years ago

This is still WIP as I plan to cleanup more more imports, but does this look about right for how you're wanting to clean up imports that are in the libc crate? The alternative is I just do use libc::fncl instead.

Akeboshiwind commented 4 years ago

@samuela You mention here that refactoring librb::size_t would be a pain. I've come across that while trying to refactor the libc::write imports :/

Would the following be fine for refactoring the size_t out of the libc::write call? (from coreutils/tee.rs):

      libc::fwrite(
        bb_common_bufsiz1.as_mut_ptr() as *const libc::c_void,
-       1i32 as size_t,
-       c as size_t,
+       1,
+       c as usize,
        *fp,
      );
samuela commented 4 years ago

@Akeboshiwind Yeah, that looks like a fair substitution to me.

The real problem here is that libc is based on size_t being usize which makes sense but c2rust wants to maintain absolute semantic equivalence which means compiling size_t to be the size that it just so happens to be on the machine that's running c2rust. So anyways that's how we got here. I think moving forward the trick will be to get c2rust refactor to fix everything en masse but I haven't figure out how to do that just yet.

Akeboshiwind commented 4 years ago

Cleaning up the imports for write and fwrite ended up being a bit bigger than I expected so I'll unmark this PR as WIP and make another PR for that change

samuela commented 4 years ago

Hmm looks like something in this merge broke the build (even though it passed at 859e74a): https://github.com/samuela/rustybox/runs/373956420#step:3:62. @Akeboshiwind Do you have any sense what's broken?

Akeboshiwind commented 4 years ago

Hmm, I've not looked too far into it yet but it looks like it was something in the latest nightly build.

It looks like your CI builds with the latest nightly version, is that advisable? Wouldn't it be better to stick to the latest stable release unless we need features a nightly?

Akeboshiwind commented 4 years ago

Although this particular error seems quite useful in clearing up some weird casting that's left over from c2rust.

samuela commented 4 years ago

Unfortunately c2rust emitted a bunch of code that uses nightly features, so we're kind of stuck with that part. I'll take a look and see if I can't fix these things.

Akeboshiwind commented 4 years ago

I'm mid way through making a PR to fix the issues if you don't mind waiting ;)

On Sun, 5 Jan 2020, 20:26 samuela, notifications@github.com wrote:

Unfortunately c2rust emitted a bunch of code that uses nightly features, so we're kind of stuck with that part. I'll take a look and see if I can't fix these things.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/samuela/rustybox/pull/15?email_source=notifications&email_token=ACD2NAWHNSJRSUG5XBTA7KDQ4I7BBA5CNFSM4KABYWJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEID6YKQ#issuecomment-570944554, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACD2NATU7VGOVOSEF6EYC73Q4I7BBANCNFSM4KABYWJQ .

samuela commented 4 years ago

Oh, perfect!