Re: [platform-testers] texinfo-6.6.90 pretest

[Hans-Bernhard Bröker]

Am 18.09.2019 um 20:07 schrieb Gavin Smith:> On Tue, Sep 17, 2019 at
7:23 PM Hans-Bernhard Bröker
> <address@hidden> wrote:

>> Unfortunately the message text does not express this very well: it fails
>> to express what's wrong about its stated observation.  Which is that the
>> size argument really _must_ depend on the size of `dest', not `src'.

> Why must it depend on the size of the destination?

Because otherwise using strncat is just wasteful.

It makes the copying operation more difficult, because the copy loop has
to check for both the source string's end, and the exhaustion of the
given length.  If, like in these cases, we already know both termination
conditions will hit simultaneously, it's a waste to have the copy loop
check both of them on every iteration.

> strncat is not being used here as a "safe" version of strcat.

Then it should not be used at all, because as-is it just adds waste and
confusion for zero gain.

> The length argument is saying how many bytes to copy from the source,
> which I see nothing wrong with.

In net effect, each of these strncat()s copies one byte _more_ from the
source than the given length argument indicates: the terminating '\0'.
Coincidentally, if one includes that extra byte in the length argument,
all but one these warnings go away, if only because the compiler does
not figure out that a literal 2 is just the length of " " including the
terminator.

OTOH if what these calls are supposed to be doing is really is "copy n
bytes", it would be more honest to replace them by variations of

        memcpy (dest + strlen(dest), src, n);

(with some actual book-keeping to replace the repeated determination of
strlen(dest), obviously).

> In some cases the length argument is being given in a variable that is
> also used elsewhere.

Actually, of the warnings in question, that's only one case (line 1634).

And anyway, just because the strlen(src) happens to be at hand already,
that doesn't make strncat() more applicable.

> I'm not minded to rewrite it all to silence a
> compiler warning, risking introducing bugs in the process.

Nobody said anything about rewriting "it all".  The change I'm
advocating is as trivially easy as removing one 'n', one ',' and the
third argument from the call, per warning location.  Frankly, if that's
too risky already, I'm at a loss for words.