-
Notifications
You must be signed in to change notification settings - Fork 779
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove the need of -Wno-stringop-overflow
warning suppression
#11357
Conversation
The problem was that GCC complained for possibly incorrect usage of strncat due to the last argument being equal to the source string length. The last argument of strncat is supposed to be the remaining free space in the destination buffer. Replaced the usage of strncat with the ATS function ink_strlcat.
[approve ci] |
It looks like there is a possible similar incorrect usage of |
Hmm, I built all of the plugins and there was no other complains from GCC 13.2 on Debian. It seems to me that in the Maybe it'll be better if I change all of the remaining 5 usages of
with Or the suppression can be left as it is at global level? Or another option is the suppression to be set in the |
I don't mind leaving the warning suppression. Sounds like the change is enough for you, and I'm not going to have you fix all the places. We can remove the warning suppression later when we are ready to do so. But if you are willing to make additional changes for the plugin code, please use TSstrlcat, which is exactly the same as ink_strlcat but it's exported for plugins. |
The change is to use TSstrlcat instead of strncat. It seems to me the GCC analyzer is wrong in this case and it issues the warning because of such usage: str_concat(&left[loff], (left_size - loff), "/", 1); and the str_concat internally does std::min(dst_len, src_len) for the strncat size argument. So GCC sees the possibility that the last argument of strncat can be equal to the legnth of the source argument and issues the warning.
I replaced the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Cherry-picked to v10.0.x |
* Remove the need of -Wno-stringop-overflow warning suppression The problem was that GCC complained for possibly incorrect usage of strncat due to the last argument being equal to the source string length. The last argument of strncat is supposed to be the remaining free space in the destination buffer. Replaced the usage of strncat with the ATS function ink_strlcat. * Fix bogus warning for strncat usage in s3_auth plugin given by GCC 11 The change is to use TSstrlcat instead of strncat. It seems to me the GCC analyzer is wrong in this case and it issues the warning because of such usage: str_concat(&left[loff], (left_size - loff), "/", 1); and the str_concat internally does std::min(dst_len, src_len) for the strncat size argument. So GCC sees the possibility that the last argument of strncat can be equal to the legnth of the source argument and issues the warning. (cherry picked from commit 7220d70)
The problem was that GCC complained for possibly incorrect usage of
strncat
due to the last argument being equal to the source string length. The last argument ofstrncat
is supposed to be the remaining free space in the destination buffer.Replaced the usage of
strncat
with the ATS functionink_strlcat
.More info about this can be found in this issue