replace strncpy with memcpy when copying full string

[emaste]

Summary

memcpy(dst, src, strlen(src)) is equivalent to strncpy(dst, src, strlen(src)) but may be slightly more efficient, and avoids a string truncation warning with gcc9

[emaste]

Using memcpy avoids the warning and makes use of the already-calculated strlen, and I'm inclined to commit this as a first step. That said, I'd like to replace all of these with a better idiom.

[emaste]

And note there is still one error after this change:

cc -O2 -Wno-error -DWITH_PE=1 -I. -I/tmp/cirrus-ci-build/elfcopy -I/tmp/cirrus-ci-build/elfcopy/../common -I/tmp/cirrus-ci-build/elfcopy/../libelf -I/tmp/cirrus-ci-build/elfcopy/../libelftc -I/tmp/cirrus-ci-build/elfcopy/../libpe  -Wall -Wstrict-prototypes -Wmissing-prototypes -Wpointer-arith -Wno-uninitialized -Wreturn-type -Wcast-qual -Wpointer-arith -Wwrite-strings -Wswitch -Wshadow -Werror   -c pe.c
pe.c: In function 'create_pe':
pe.c:177:3: error: 'strncpy' specified bound 8 equals destination size [-Werror=stringop-truncation]
   strncpy(psh.sh_name, name, sizeof(psh.sh_name));
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
*** Error code 1

[emaste]

In D23531#516575, @emaste wrote:

This GCC warning is bogus - this case is an example (one of the few) where strncpy is actually the right function.

This comment is meant in general - strncpy()'s semantics (truncating without \0 if the output buffer is not large enough, otherwise zero-filling the entire remaining buffer) has limited applicability, except for kernel APIs and file formats that have fixed length string buffers.

[markj]

I can't see any problems.

ar/write.c

983

Why not cache strlen(name) in a local var? Otherwise we are making the same call three times in a row.

elfcopy/archive.c

181

Ditto here.

[cem]

ar/write.c

983

Strlen() should be a pure and/const function and the compiler is free to cache the result itself. I wouldn’t object to making it explicit, but I don’t believe this should generate 3 calls today.

[emaste]

ar/write.c

983

I think reworking this to avoid multiple strlen calls (regardless of whether the compiler can elide some or not) is worthwhile, but I'd do that as a later change.

[markj]

ar/write.c

983

Doesn't that only work if the compiler can prove that the memory pointed to by name isn't modified through an aliasing pointer? I'd expect that the third call below can be elided because memcpy() qualifies src and dst with restrict, but I don't see how the compiler can only call strlen() once.

[cem]

ar/write.c

983

I don't see how this routine could modify name through an aliasing pointer between the first strlen call and the last.

If we take the false case of the while loop above the first time, we cannot have modified name.

If we take the true case one time or more, and name aliased s_sn, name would become UB as soon as we called realloc for the first time. As this is UB, the compiler is free to assume it is impossible.

If we take the true case one time or more, and name does not alias s_sn, it won't alias s_sn after realloc(), which is guarantee to return unique objects.

So the compiler may not be smart enough to prove it, but I don't think it is possible for an aliased modification of name here.

(I also don't know if marking restrict on the parameters to this function (bsdar and name) would be sufficient to inform the compiler, since the potential alias is with bsdar->s_sn's contents, not bsdar itself.)