Switch from strncpy to memcpy in strcpy0 function

[carmiker]

This pull request fixes a stringop-truncation warning.
ini.c:78:5: warning: ‘strncpy’ output may be truncated copying 49 bytes from a string of length 199 [-Wstringop-truncation]

[benhoyt]

Is this gcc? What version of compiler are you using?

[carmiker]

I am indeed using gcc (9.2.1), building this as part of a C project using the following flags:
-std=c99 -Wall -Wextra -pedantic

[benhoyt]

Do you know why exactly it's giving that warning? I think this is a legitimate, safe use of strncpy as I'm passing in the length and ensuring the NUL terminator is there in the surrounding code in strncpy0.

[carmiker]

I believe the compiler is under the impression that the maximum length of the string passed in is 199 bytes. I have not been able to discern why this could ever be the case.

For the record, clang 8.0.1 does not exhibit this warning. However, memcpy is probably a better option regardless. Just ask Ulrich Drepper.

[benhoyt]

Can you help me understand why memcpy is a better option regardless? To me it seems like strncpy (plus the NUL handling I have) is better as it has a more specific char* signature rather than void*. Or point me to some docs here. Happy to change this, just want to understand the why.

[carmiker]

Theoretically, memcpy is faster as it copies in blocks vs byte by byte. Of course, for this use case that is really negligible. Since you're forcing the last byte to be a \0 anyway, there is no effective difference in the result. I probably should have cast the destination buffer to (char*) in my pull request now that you mention it. If you want to close this without merging I won't be insulted; it is a very small detail. I just want to silence a warning in my project that other people may end up seeing in the future as well.

I will also add that I cannot generate the warning on another system that uses the same version of gcc, but is musl libc based vs glibc based.

Another thing to add here: I compared the generated assembly on godbolt and found no differences whatsoever. So, feel free to close this if you wish. The fact that there is a warning at all is probably the bug.

[mupfdev]

Hello,
there might be a solution in between: GCC diagnostic pragma to suppress the warning at this line of code whenever GCC is used:

https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html

It should even work if you enable and disable this pragma before and after the include of ini.h. As a temporary solution until a decision is made.

However, this probably isn't good practice and as you said: in the end it shouldn't make a difference. The compiler probably generates the exact same instructions.

Best regards, Michael

[benhoyt]

Thanks -- appreciate the explanation. I think I'll skip merging this. It seems to me the warning is erroneous, and strncpy to me conveys more semantic sense here. Plus, I'm a little bit wary about changing it as the current version has been "battle tested" in a bunch of different compilers by now.

[benhoyt]

Finally caved :-) and fixed this in 8fe4b21 -- thanks for your help several months ago!

[carmiker]

Yes, yes... jolly good!

[benhoyt]

Plus, I'm a little bit wary about changing it as the current version has been "battle tested" in a bunch of different compilers by now.

Heh, it seems I was right to be wary. This change actually introduced a "read from invalid memory" issue, see #107. I've fixed in https://github.com/benhoyt/inih/releases/tag/r51, hopefully in a way that avoids the gcc warning too.

[bostjan]

For what is worth, I've never seen this issue with gcc + glibc, and Snoopy has been building inih with -Wall -Wextra -std=c99 -Wpedantic/-pedantic for a very long time now. I can't reproduce it with GCC 9.3.0 + -Wstringop-truncation flag right now either.

@carmiker, on what OS(+version) were you compiling inih that resulted in the error reported here?