Proposition for removing the global suppression `-Wno-stringop-overflow` from the build
Hi there,
As far as I checked the only place where this warning is raised is here:
/z/x3me-ocn/src/iocore/net/OCSPStapling.cc:1166:12: warning: ‘char* strncat(char*, const char*, size_t)’ specified bound 1 equals source length [-Wstringop-overflow=]
1166 | strncat(url->end(), "/", 1);
Here is the code in question:
if (url->buf()[url->size() - 1] != '/') {
strncat(url->end(), "/", 1);
url->fill(1);
}
Note that the url buffer size is calculated above in such a way that there is a guarantee that it's big enough for this concatenation.
This type of GCC warning is a bit bogus.
However, I think with a slight change the code here can be made better/safer and at the same time the global suppression of this warning can be removed.
Although it's not very obvious (IMO) from the man page of strncat, the last size_t argument is supposed to specify the remaining space in the destination buffer.
So, I think this code is better written as:
if (url->buf()[url->size() - 1] != '/') {
strncat(url->end(), "/", url->write_avail());
url->fill(1);
}
or
if (url->buf()[url->size() - 1] != '/') {
written = ink_strlcat(url->end(), "/", url->write_avail());
url->fill(written);
}
What do you think?
The proposed code looks good, and I'm +1 on removing the suppression. I'd go with ink_strlcat because use of strncat isn't common in ATS codebase. Would you make a Pull Request?
I'm OK to do a pull request but I'm not sure against which branch to do it?
A Pull Request against master would be great. We'll take care of other branches.