Re: strscpy

[Maxime Villard <max%m00nbsd.net@localhost>]

Le 19/05/2020 à 07:44, Martin Husemann a écrit :
> On Tue, May 19, 2020 at 07:31:39AM +0200, Maxime Villard wrote:
> > copystr() has a small dozen of users.
>
> nxr only finds two - can we please fix that and have a version that does
> a full index?

For me, NXR reports about a dozen.

> > 	https://m00nbsd.net/garbage/libkern/strscpy.diff
>
> I find that function totally ugly, (a) due to the return value overload
> and (b) why would any caller care to tell the two possible error values
> apart? and (c) it leaves the destination non-null terminated in one of
> the failure cases.

(a) As far as can be seen on NXR, 99% of the callers don't care about the
return value. It is not like overloading the return value is of particular
concern here.
(b) Why not? If it fails for two different reasons, it is not irrelevant
to indicate the reason.
(c) Which is the case where a garbage length was supplied. What do you
want to do here? Fill with garbage? The goal of the INT_MAX check is to
avoid precisely that.

Having said that, I do still have a small doubt about whether it is
desired to check against INT_MAX. After all, if a caller passes garbage
in the size argument, it is likely to misbehave after strscpy returns
anyway.

An earlier version of my patch didn't have the check on INT_MAX, and was
returning -1 on error. That is:

	ssize_t
	strscpy(char *dst, const char *src, size_t size)
	{
		size_t i;
	
		if (__predict_false(size == 0))
			return -1;
	
		for (i = 0; i < size; i++) {
			if ((dst[i] = src[i]) == '\0')
				return i;
		}
	
		dst[size - 1] = '\0';
		return -1;
	}

This is less linux-y and closer to the posix conventions.

Maxime