UCStr::from_ptr_with_nul & inconsistent documentation + behaviors around nulls

[MaulingMonkey]

The documentation here for UCStr::from_ptr_with_nul is a little self-contradictory - "Safety" claims the pointer mustn't be null, but then "Panics" documents safe behavior (panicing) on null, but then the implementation doesn't check for null and may or may hit debug-only asserts inside of std:

widestring-rs/src/ucstr.rs

Line 132 in e7236b6

/// `p` must be non-null, even for zero `len`.

widestring-rs/src/ucstr.rs

Line 139 in e7236b6

/// This function panics if `p` is null or if a nul value is not found at offset `len` of `p`.

widestring-rs/src/ucstr.rs

Lines 148 to 152 in e7236b6

	pub unsafe fn from_ptr_with_nul<'a>(p: *const C, len: usize) -> &'a Self {
	assert!(*p.add(len) == UChar::NUL);
	let ptr: *const [C] = slice::from_raw_parts(p, len + 1);
	&*(ptr as *const UCStr<C>)
	}

Additionally, while UCStr::from_ptr_with_nul doesn't scan for nuls, it appears UCString::from_ptr_with_nul does (and will truncate) - and also handles the len=0 ptr=null case without panicing at all:

widestring-rs/src/ucstring.rs

Lines 587 to 597 in e7236b6

	pub unsafe fn from_ptr_with_nul(
	p: *const u16,
	len: usize,
	) -> Result<Self, MissingNulError<u16>> {
	if len == 0 {
	return Ok(UCString::default());
	}
	assert!(!p.is_null());
	let slice = slice::from_raw_parts(p, len);
	UCString::from_vec_with_nul(slice)
	}

calls:

widestring-rs/src/ucstring.rs

Lines 181 to 191 in e7236b6

	pub fn from_vec_with_nul(v: impl Into<Vec<C>>) -> Result<Self, MissingNulError<C>> {
	let mut v = v.into();
	// Check for nul vals
	match v.iter().position(|&val| val == UChar::NUL) {
	None => Err(MissingNulError { inner: Some(v) }),
	Some(pos) => {
	v.truncate(pos + 1);
	Ok(unsafe { UCString::from_vec_with_nul_unchecked(v) })
	}
	}
	}

Should I create a PR for UCStr to try and return &[UChar::NUL] for the length 0 case? (Maybe UCStr can gain a Default impl?)
Should it scan/truncate too (would change the result of str.len())? Perhaps matching function signatures?

The inconsistent "Safety" vs "Panics" documentation crops up in multiple places - should I try and drop this text from all the "Safety" sections where p is already null-checked soundly and documented to be null-checked under "Panics"?

p must be non-null.

[starkat99]

Not surprised some inconsistencies have popped up, there was some significant internal refactoring. A PR to correct these would be welcome.

Should I create a PR for UCStr to try and return &[UChar::NUL] for the length 0 case? (Maybe UCStr can gain a Default impl?) Should it scan/truncate too (would change the result of str.len())? Perhaps matching function signatures?

In general, _unchecked methods should not scan/truncate, and methods that are passed a len parameter should scan and error on missing/unexpected nuls depending on signature. Methods that receive only the pointer should scan and truncate without error. This should be true for both UCStr and UCString. Getting the zero length behavior consistent would be useful