github-starred/libxlsxwriter

Fork 0

mirror of https://github.com/jmcnamara/libxlsxwriter.git synced 2026-05-15 14:15:54 -06:00

[GH-ISSUE #192] gcc 8 warnings #154

New issue

Closed

opened 2026-05-05 11:48:12 -06:00 by gitea-mirror · 8 comments

gitea-mirror commented

2026-05-05 11:48:12 -06:00

Owner

Originally created by @jeroen on GitHub (Aug 21, 2018).
Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/192

Originally assigned to: @jmcnamara on GitHub.

    Found the following significant warnings:
     libxlsxwriter/xmlwriter.c:156:17: warning: ‘strncat’ specified bound 5 equals source length [-Wstringop-overflow=]
     libxlsxwriter/xmlwriter.c:160:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=]
     libxlsxwriter/xmlwriter.c:164:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=]
     libxlsxwriter/xmlwriter.c:168:17: warning: ‘strncat’ specified bound 6 equals source length [-Wstringop-overflow=]
     libxlsxwriter/xmlwriter.c:198:17: warning: ‘strncat’ specified bound 5 equals source length [-Wstringop-overflow=]
     libxlsxwriter/xmlwriter.c:202:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=]
     libxlsxwriter/xmlwriter.c:206:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=]
     tmpfileplus/tmpfileplus.c:172:3: warning: ‘strncpy’ specified bound 4097 equals destination size [-Wstringop-truncation]

Originally created by @jeroen on GitHub (Aug 21, 2018). Original GitHub issue: https://github.com/jmcnamara/libxlsxwriter/issues/192 Originally assigned to: @jmcnamara on GitHub. ``` Found the following significant warnings: libxlsxwriter/xmlwriter.c:156:17: warning: ‘strncat’ specified bound 5 equals source length [-Wstringop-overflow=] libxlsxwriter/xmlwriter.c:160:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=] libxlsxwriter/xmlwriter.c:164:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=] libxlsxwriter/xmlwriter.c:168:17: warning: ‘strncat’ specified bound 6 equals source length [-Wstringop-overflow=] libxlsxwriter/xmlwriter.c:198:17: warning: ‘strncat’ specified bound 5 equals source length [-Wstringop-overflow=] libxlsxwriter/xmlwriter.c:202:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=] libxlsxwriter/xmlwriter.c:206:17: warning: ‘strncat’ specified bound 4 equals source length [-Wstringop-overflow=] tmpfileplus/tmpfileplus.c:172:3: warning: ‘strncpy’ specified bound 4097 equals destination size [-Wstringop-truncation] ```

gitea-mirror

2026-05-05 11:48:12 -06:00

closed this issue
added the
bug

ready to close
labels

gitea-mirror commented

2026-05-05 11:48:18 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 21, 2018):

What is the exact gcc --version?

The warnings look spurious to me but I'll install gcc 8.2 and check.

@jmcnamara commented on GitHub (Aug 21, 2018): What is the exact `gcc --version`? The warnings look spurious to me but I'll install gcc 8.2 and check.

gitea-mirror commented

2026-05-05 11:48:22 -06:00

Author

Owner

@jeroen commented on GitHub (Aug 21, 2018):

The warnings appear both on Fedora 28 and Debian Buster with the current gcc: https://cran.r-project.org/web/checks/check_results_writexl.html

Yes the warnings are silly, but my repo maintainers insist that all code compiles without warnings even with -Wall -pedantic. The fix is easy: you just need to increase the buffer size by 1 so that there is no theoretical case where the terminating \0 gets dropped when copying a string (even if this will never actually happen).

@jeroen commented on GitHub (Aug 21, 2018): The warnings appear both on Fedora 28 and Debian Buster with the current gcc: https://cran.r-project.org/web/checks/check_results_writexl.html Yes the warnings are silly, but my repo maintainers insist that all code compiles without warnings even with `-Wall -pedantic`. The fix is easy: you just need to increase the buffer size by 1 so that there is no theoretical case where the terminating `\0` gets dropped when copying a string (even if this will never actually happen).

gitea-mirror commented

2026-05-05 11:48:24 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 21, 2018):

The fix is easy: you just need to increase the buffer size by 1 so that there is no theoretical case where the terminating \0 gets dropped when copying a string (even if this will never actually happen).

Nope. That isn't the case if you look at the code and the buffers involved (at least for the xmlwriter warnings. The tmpfileplus isn't my code but it looks like it is properly terminating the buffer).

Older gccs, clang, ICC, TCC, scan-build and coverity don't complain about this. So I still think it is spurious and also probably not fixable.

@jmcnamara commented on GitHub (Aug 21, 2018): > The fix is easy: you just need to increase the buffer size by 1 so that there is no theoretical case where the terminating \0 gets dropped when copying a string (even if this will never actually happen). Nope. That isn't the case if you look at the code and the buffers involved (at least for the xmlwriter warnings. The tmpfileplus isn't my code but it looks like it is properly terminating the buffer). Older gccs, clang, ICC, TCC, scan-build and coverity don't complain about this. So I still think it is spurious and also probably not fixable.

gitea-mirror commented

2026-05-05 11:48:26 -06:00

Author

Owner

@jeroen commented on GitHub (Aug 21, 2018):

Well it's not entirely spurious. The definition of strncat is: Appends the first num characters of source to destination, plus a terminating null-character. However you are doing this:

#define LXW_AMP  "&amp;"
strncat(p_encoded, LXW_AMP, sizeof(LXW_AMP) - 1);

For example here LXW_AMP is 6 bytes and you strncat it with a limit of 5, so strncat() never makes it to the null terminator of the input string, and it warns about that. Maybe you should use memcpy()?

Older gccs, clang, ICC, TCC, scan-build and coverity don't complain about this.

Yes I know, gcc 8 introduces a bunch of new compiler warnings that try to enforce some best practices in preventing buffer overflows when it comes to copying strings. We've seen similar warnings in many libraries, they are usually easy to fix.

@jeroen commented on GitHub (Aug 21, 2018): Well it's not entirely spurious. The definition of strncat is: *Appends the first num characters of source to destination, plus a terminating null-character.* However you are doing this: ```c #define LXW_AMP "&" strncat(p_encoded, LXW_AMP, sizeof(LXW_AMP) - 1); ``` For example here `LXW_AMP` is 6 bytes and you `strncat` it with a limit of 5, so `strncat()` never makes it to the null terminator of the input string, and it warns about that. Maybe you should use `memcpy()`? > Older gccs, clang, ICC, TCC, scan-build and coverity don't complain about this. Yes I know, gcc 8 introduces a bunch of new compiler warnings that try to enforce some best practices in preventing buffer overflows when it comes to copying strings. We've seen similar warnings in many libraries, they are usually easy to fix.

gitea-mirror commented

2026-05-05 11:48:28 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 21, 2018):

For example here LXW_AMP is 6 bytes and you strncat it with a limit of 5, so strncat() never makes it to the null terminator of the input string, and it warns about that.

That's not what it is complaining about though. At least as far as I can see. The -Wstringop-overflow= error is about the destination buffer.

Anyway, I'll need to do some testing with gcc8 to see what can be done.

@jmcnamara commented on GitHub (Aug 21, 2018): > For example here LXW_AMP is 6 bytes and you strncat it with a limit of 5, so strncat() never makes it to the null terminator of the input string, and it warns about that. That's not what it is complaining about though. At least as far as I can see. The `-Wstringop-overflow=` error is about the destination buffer. Anyway, I'll need to do some testing with gcc8 to see what can be done.

gitea-mirror commented

2026-05-05 11:48:30 -06:00

Author

Owner

@jeroen commented on GitHub (Aug 21, 2018):

The -Wstringop-overflow= error is about the destination buffer.

The error actually says it is about the source ‘strncat’ specified bound 5 equals source length

@jeroen commented on GitHub (Aug 21, 2018): > The -Wstringop-overflow= error is about the destination buffer. The error actually says it is about the source `‘strncat’ specified bound 5 equals source length`

gitea-mirror commented

2026-05-05 11:48:31 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 21, 2018):

Okay. That is different. In that case it may be valid. Either way I'll look in to it.

@jmcnamara commented on GitHub (Aug 21, 2018): Okay. That is different. In that case it may be valid. Either way I'll look in to it.

gitea-mirror commented

2026-05-05 11:48:32 -06:00

Author

Owner

@jmcnamara commented on GitHub (Aug 22, 2018):

I've push a fix for this to master. I also fixed some warnings in the third party libraries that are embedded in the source tree.

I tested it with gcc 8.2.0 and 9.0.0 so we should be warning free for a while.

Thanks for the report. Let me know if you need it tagged in a release for cran and I can do that in the next few days.

@jmcnamara commented on GitHub (Aug 22, 2018): I've push a fix for this to master. I also fixed some warnings in the third party libraries that are embedded in the source tree. I tested it with gcc 8.2.0 and 9.0.0 so we should be warning free for a while. Thanks for the report. Let me know if you need it tagged in a release for cran and I can do that in the next few days.

gitea-mirror referenced this issue

2026-05-05 12:15:36 -06:00

[PR #154] [CLOSED] CMake/MSVC - build with support for Unicode #426