> On Sep 6, 2020, at 1:03 PM, Florian Weimer <fwei...@redhat.com> wrote:
> There is no reason to use strncpy. At least on Linux, the struct field
> needs to be null-terminated, and you need to compute the length for the
> length check. So you might as well use memcpy with the length plus one
> (to copy the null terminator). You can keep using strncpy, and the
> warning should be gone (because GCC will recognize a dominating strlen
> check), but it's not necessary.
>
> On current GNU/Linux, the most common structs now have the appropriate
> annotations, so you get the strncpy warnings only in cases where there
> is an actual truncation bug.
[Keep in mind that gcc on recent GNU/Linux is not the only platform
this code base needs to worry about. Though it is the one that is
driving us a bit batty with its varying warnings here.]
I'm not sure whether you are suggesting one should never use strncpy
anywhere, or whether there is some specific place(s) from this webrev
you are suggesting it should be avoided?
I agree there are some uses of strncpy that look at least somewhat
suspect. Without further analysis it's hard to know whether there is
really a problem with truncation (can it occur at all, and is it a
problem if it does). Figuring that out seems somewhat beyond the
scope of what this change is trying to do. Please file bugs about
problematic uses. (A bug of the form "don't use strncpy" might not
get quick action.)
And strlen is not even necessarily the best solution, as it likely
introduces an additional otherwise unnecessary string traversal. For
example, getFlags could be changed to reject an overly long ifname,
without using strlen, thusly:
strncpy(if2.ifr_name, ifname, sizeof(if2.ifr_name));
if (if2.ifr_name[sizeof(if2.ifr_name) - 1] != '\0') {
return -1;
}
Unfortunately, gcc10 -Wstringop-truncation whines about this entirely
reasonable code.
