On Mon, Apr 26, 2021 at 11:10 PM Samuel Thibault <samuel.thiba...@gnu.org> wrote: > Err, but wouldn't the compiler be able to determine that the size was > properly computed, and avoid emitting a false-positive warning?
It is my understanding, based on https://gcc.gnu.org/bugzilla//show_bug.cgi?id=88059, that GCC does not do any sophisticated analysis here, and just warns about any case where the specified length depends on the source size. Which makes sense to me, because either the destination buffer size depends on the source string length, in which case you can be sure it fits and don't need strncpy, or it does not depend on the source string length, in which case the string might not fit and you'd use strncpy, passing the destination buffer size. > The compiler emitting a warning looks to me like a sign that there might > really be something subtly wrong. Actually sending the output of the > compiler on the list would help us to know more about it. Here are the warnings I get (gcc --version is gcc (Debian 10.2.1-6+hurd.1) 10.2.1 20210110): lib.c: In function ‘make_filepath’: lib.c:154:3: warning: ‘strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=] 154 | strncpy (filepath, path, length); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lib.c:149:12: note: length computed here 149 | length = strlen (path) + strlen (filename) + 2; | ^~~~~~~~~~~~~ lib.c:155:3: warning: ‘strncat’ specified bound depends on the length of the source argument [-Wstringop-overflow=] 155 | strncat (filepath, filename, strlen (filename)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ options.c: In function ‘argp_parse_common_options’: options.c:77:5: warning: variable ‘ulfs_match’ set but not used [-Wunused-but-set-variable] 77 | ulfs_match = 0, ulfs_priority = 0; | ^~~~~~~~~~ stow.c: In function ‘stow_diradd’: stow.c:290:7: warning: ‘strncpy’ output truncated before terminating nul copying as many bytes from a string as its length [-Wstringop-truncation] 290 | strncpy (tmp, dir, dir_len); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ stow.c:275:13: note: length computed here 275 | dir_len = strlen(dir); | ^~~~~~~~~~~ The last one is indeed a false positive, because we (with my previous patch merged) now null-terminate the resulting string explicitly. -- Sergey
