rafat
Suspicious code triggers warnings
When enabling much more warnings, I get the following (with gcc version 11.4.0 and Apple clang 12.0.0):
- [ ] erfunc.c: warning: floating constant truncated to zero [-Woverflow] for lines 333 and 340
- line 333 -> } else if( x < 1.0e-100 && x > 1.0e-1000 ) {
- line 340 -> } else if (x <= 1.0e-1000) {
- Are such comparisons required for some architectures that can handle such numbers? (the minimum value for a float is 2^31 - 1)
- [X] ctsa.h: warning: ISO C++ forbids zero-size array ‘params’ for lines 119, 146, 179, 199
- all lines have the following: -> double params[0];
- Is it something that should be replaced by a simple double? Or by double params[] ?
- Fixed with commit 80c4be613111cd344941fd0b964e2fcf454a1b96
- [ ] warning: self-comparison always evaluates to true
- at seastest.c:673 "if (fit->rank != fit->p && fit->rank == fit->rank)" I think this one can safely be removed (no way it would evaluate to false
- at autoutils.c: line 131 "if (dodiff != dodiff) return d;" and line 155 "if (dodiff != dodiff)". From my point of view, it would only evaluate to false if they are undefined (underflow, overflow...). I'm not even sure this could be possible for integers and I doubt this is the goal of these lines...
- [ ] non-void function does not return a value
- at boxcox.c:68 Here I have no suggestions on how to fix it. Is the function even called anywhere?
2024-12-03: I have edited this issues to add more warnings as well as checkboxes so what has been addressed is marked as such
double params[0] is an old C convention and I am not sure how compatible it is with GCC 11.x so I'll have to check. As far as the floating point precision is concerned one way is to remove the lines 333-339 and set 340 to "} else if (x <= Float_Precision) {" but it might be a good idea to revisit the algorithm, which I haven't done in a few years, before doing that.
On Fri, Oct 25, 2024 at 10:06 PM Mathias Bavay @.***> wrote:
When enabling much more warnings, I get the following (with gcc version 11.4.0):
- erfunc.c: warning: floating constant truncated to zero [-Woverflow] for lines 333 and 340
- line 333 -> } else if( x < 1.0e-100 && x > 1.0e-1000 ) {
- line 340 -> } else if (x <= 1.0e-1000) {
- Are such comparisons required for some architectures that can handle such numbers? (the minimum value for a float is 2^31 - 1)
- ctsa.h: warning: ISO C++ forbids zero-size array ‘params’ for lines 119, 146, 179, 199
- all lines have the following: -> double params[0];
- Is it something that should be replaced by a simple double? Or by double params[] ?
— Reply to this email directly, view it on GitHub https://github.com/rafat/ctsa/issues/16, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAI3RO4X3IPY2ZKZNLNVHXTZ5JXPRAVCNFSM6AAAAABQTUJVN6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGYYTINJZGA4DGMI . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Some context for the double params[0] code snippet: as part of a structure (which is the case in ctsa), this is a flexible array. In theory, this should be invalid from C89 onward, but GCC has maintained it as a gcc extension (although with a warning at least in some gcc versions). The ISO C99 syntax for flexible arrays should be double params[]. So we should be able to simply remove the 0 and get the same behavior with a standard compliant syntax.
For more information:
- GCC documentation on flexible arrays;
- a detailed explanation on Quora by a Linux kernel developer