str::buf_as_slice includes (and reads) uninitialized memory in the slice

[brson]

As long as string slices expect to have an extra byte at the end this function cannot be expressed without copying. Here's how it's written:

1964-    /// Form a slice from a *u8 buffer of the given length without copying.
1965:    pub unsafe fn buf_as_slice<T>(buf: *u8, len: uint,
1966-                              f: fn(v: &str) -> T) -> T {
1967-        let v = (buf, len + 1);
1968-        assert is_utf8(::cast::reinterpret_cast(&v));
1969-        f(::cast::transmute(move v))
1970-    }
1971-

That extra byte in len + 1 is just random memory, which is_utf8 immediately reads.

[graydon]

curiously it seems nobody uses this at the moment; I agree it shouldn't read possibly-invalid memory, but can we either say len should be one-longer than the intended length, and/or len+1 must be valid memory (as a precondition)? Or is this a general relative of the 'maybe str should not include a trailing byte' bug?

[graydon]

non-critical for 0.6, de-milestoning

[graydon]

non-critical for 0.6, de-milestoning

[brson]

@graydon We can state some precondition in the docs, yes. It will be error prone, but I suppose that's the nature of unsafe.

Not really related - with region pointers I think these types of functions that temporarily cast an unsafe pointer to another type in a closure aren't a best practice. We ought to just fabricate a region (would be nice to have a dedicated 'unsafe region maybe?) and return it.

[bblum]

@brson An "unsafe" region wouldn't be necessary for that; you'd want to write fn buf_as_slice<'a>(&'a str, uint len) -> &'a str.

But I suggest this function shouldn't be removed entirely (perhaps replaced with a safe copy_slice(&str, uint) -> ~str). The only place it's used is in uv_error_to_io_error, in a helper function called c_str_to_static_slice -- which itself should be moved to the str module and written without using buf_as_slice.

[bblum]

I also note that the function is "currently safe" because its only caller ensures that there's a null in the appropriate spot.