Proposition for removing the global suppression -Wno-stringop-overflow from the build

[freak82]

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?

[maskit]

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?

[freak82]

I'm OK to do a pull request but I'm not sure against which branch to do it?

[maskit]

A Pull Request against master would be great. We'll take care of other branches.