Make `strlen` more like musl version

[bombless]

I think there are 2 tricks here:

• If you read from memory, it is best that the memory address is aligned to word-boundary
• If you check a string of memory and you can check N bytes within the same time, you are N times faster than you check one byte at a time

[bombless]

I didn't add tests directly but I do did some unit tests: https://play.rust-lang.org/?gist=1953956b2b941525bb89abcda6925db1&version=nightly&backtrace=0&run=1

[anp]

Thanks for the PR! I need to document this, but I'm relying on the test suite used for musl to check any changes. If you're on Linux (or a Linux VM), you should be able to run ./build_and_test.sh from the project root directly, which will build musl and rusl, and run libc-test on the combined archived. Could you give that a try? I'll give it a go sometime soon when I have some time to clone this down.

[bombless]

Nits addressed

[bombless]

http://doc.rust-lang.org/reference.html#conditional-compilation I think we can replace these inlined functions to const if we make use of target_pointer_width

[bombless]

Ah I forgot to add tests. I'll try to run the test once I find a Linux machine, within a few days. Or feel free to take over this PR.

[bombless]

https://crates.io/crates/pointer-width made a crate to provide pointer size at compile-time not sure if it's good idea though

[eddyb]

@bombless Isn't this what std::usize::BYTES was?

[bombless]

Thanks @eddyb, but isn't this feature deprecated? https://github.com/rust-lang/rust/issues/27753

[anp]

Looks great, thanks! I'll run this through the test suite soon.

[bombless]

src/api/fcntl.c:30:1: note: each undeclared identifier is reported only once for each function it appears in src/common/runtest.c:29: ./src/api/main.exe exec failed: No such file or directory FAIL ./src/api/main.exe [status 1] gcc: error: src/api/fcntl.o: No such file or directory

Looks wierd?

[anp]

Unfortunately, musl doesn't pass all of it's own tests (here's a list of ones which fail on x86_64 with the pure C version: https://github.com/dikaiosune/rusl/blob/master/baseline_failures). The tests actually "pass" in that they don't regress any further.

The CI build failed because I just now added a script which gates the build on rustfmt having nothing to change. Could you run the patch through rustfmt 0.5.0 (cargo fmt from the root directory is easiest as it will include the configuration file) and see if CI passes then?

EDIT: Also, sorry for not giving you a heads up on the change, I just did it earlier tonight and was in the process of closing other issues.

[anp]

Also, another heads up -- I'm going to be including clippy as a build dependency, so you may wish to hold off on further attempts until I've got that sorted out and you can rebase with the changes.

[bombless]

I'll find a Linux machine tomorrow, guess that will make things more smoothly :-)

[anp]

OK, sounds good. I'll probably have clippy set up by then, so I imagine you'll want to cargo install clippy as well.

[eddyb]

I went back and looked at the C version:

#define ALIGN (sizeof(size_t))
#define ONES ((size_t)-1/UCHAR_MAX)
#define HIGHS (ONES * (UCHAR_MAX/2+1))
#define HASZERO(x) ((x)-ONES & ~(x) & HIGHS)

That's quite different from what's in this PR. In Rust, it would be:

let align = mem::size_of::<usize>();
let ones = usize::MAX / (u8::MAX as usize);
let highs = ones.wrapping_mul((u8::MAX / 2 + 1) as usize);
let has_zero = |x: usize| (x.wrapping_sub(ones) & !x & highs) != 0;

However, this can be made much simpler:

const ONES: usize  = 0x0101010101010101_u64 as usize;
const HIGHS: usize = 0x8080808080808080_u64 as usize;
let has_zero = |x: usize| (x.wrapping_sub(ONES) & !x & HIGHS) != 0;

[eddyb]

You can also use const HIGHS: usize = ONES << 7; if you want to avoid having more than one constant at a time. I also don't think align needs to be out of line, calling size_of directly should be fine.