Re: report a warning

[Gavin Smith]

On Mon, Jan 09, 2023 at 07:35:42PM +0100, Hans-Bernhard Bröker wrote:
> It's not.  It warns about a practice that has often been used to avoid
> another warning, while not actually achieving any improvement.
> 
> In a nutshell, strcat() itself has long since been frowned upon as a highly
> likely source of buffer overflows, and has triggered complaints from code
> checking tools and picky modes of modern compilers for quite a while now.
> Many got into the habit of working around these warnings by replacing
> 
>       strcat(a, b);
> 
> with some equivalent of
> 
>       size_t length_of_b = strlen(b);
>       /*...*/
>       strncat(a, b, length_of_b);
> 
> While this avoids literally calling strcat(), it offers no added safety at
> all: in situations where that strcat() call blows up, the replacement code
> behaves every bit as badly.

The code in install-info.c was not using strncat as a "safe version" of
strcat, though.

As I said in another message, it appears that everywhere strncat is
used, the length argument is used for the number of bytes to copy in
the source buffer, not for the size remaining in the destination buffer.
Whatever the original intention was behind strncat, this seems like
a reasonable way to use strncat based on its behaviour and documentation.

The line that led to the compiler warning was no different to the other
uses in the program, but it was the only one the compiler flagged up.

> Modern compiler versions detect this code pattern and report it as the silly
> thing it really is.

It was not so obviously silly as the length variable was used elsewhere
in the code.

> > The real way the code could be improved is to avoid using strcat or
> > strncat altogether.
> 
> I disagree with that.  IMHO the real solution should be to provide a less
> silly value for the third argument of this strncat().

strcat and strncat traverse the destination string each time they are
called, which is wasteful of time.

> 
> In other words the problem is not that this code is calling strncat(). It's
> that it doesn't even try to keep track of how much of the allocated buffer
> *description it has used already, so it cannot know what the third argument
> of strncat() has to be.
> 
> > However, in the referenced line in this warning, it seems that strcat could
> > be used for exactly the same thing.  I'm going to make this change
> > just so we stop getting reports.
> 
> That would most likely just exchange this warning for complaints from source
> auditing tools (like clang's "scan-build").
> 

We can't win, then.