Re: [PATCH] Implement strlcat [BZ#178]

[Paul Eggert <eggert at cs dot ucla dot edu>]

On 12/07/2015 05:57 AM, Zack Weinberg wrote:
> I will continue to insist on_exactly_
> matching the OpenBSD semantics or else not having these functions at all.

(I agree, as I think glibc shouldn't have these functions at all. That 
being said...)

If I understand the above comment correctly, you have at least three 
reasons to object to the proposed strlcpy+strlcat implementation.

First, it rejects null pointer arguments.

Second, when arguments overlap it doesn't necessarily have the same 
behavior as the OpenBSD implementation. For example, the OpenBSD strlcpy 
implementation always has well-defined behavior when source and 
destination overlap, but the proposed implementation does not. The 
situation with strlcat is more complicated; still, the OpenBSD 
implementation has well-defined behavior in cases where the proposed 
glibc implementation does not.

Third, the OpenBSD implementation declares strlcpy and strlcat to have 
__attribute__ ((__bounded__ ...)), an OpenBSD extension that generates 
warnings when compiling with gcc -Wbounded (an OpenBSD GCC option that 
is on by default). The proposed implementation doesn't do that so it by 
default does not diagnose bugs that the OpenBSD implementation does 
diagnose.

> When there is no space to write a nul-terminated string into, there is
> no such expectation.

Sometimes there *is* space; that is, the destination size is nonzero, 
but the proposed strlcat *still* doesn't store a terminating null.  
Doesn't this give you qualms?  It does me.

> if you polled a random sample of experienced C programmers
> they would react to that basically the same way they react to being told
> that signed integer overflow is undefined, i.e. "that's stupid and the
> standard should be changed."

If you polled a random sample of experienced C programmers and told them 
that neither strlcpy nor strlcat were guaranteed to null-terminate the 
result even when given valid arguments, they would also say "that's 
stupid and the spec should be changed". And they'd have a point.

> I could be convinced that the OpenBSD semantics are wrong, but only if 
> you (being the person demanding a change from those semantics)

I'm not demanding any change to the OpenBSD semantics. The OpenBSD 
implementation would conform to the spec that I'm proposing. I'm merely 
asking the documentation to permit not only the OpenBSD implementation, 
but also the NetBSD implementation that catches the stupid mistake of a 
null-pointer argument, and an implementation that also catches the 
stupid mistake of output that is not null-terminated. It's reasonable to 
have debugging implementations that catch stupid mistakes like this.

> find a
> piece of existing code that can*accidentally*  call either strlcpy or
> strlcat with a zero buffer-size argument.

OpenSSH 7.1p1 progressmeter.c on a platform where struct winsize's 
ws_col member is an int and is -1, so setscreensize sets win_size to 
zero, so refresh_progress_meter's call to strlcat(buf, " ", win_size) 
has a zero buffer-size argument. Admittedly implausible on typical 
environments, but there it is.