-
-
Notifications
You must be signed in to change notification settings - Fork 339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
lib/raster3d: fix -Wdeprecated-non-prototype compiler warnings #2901
lib/raster3d: fix -Wdeprecated-non-prototype compiler warnings #2901
Conversation
ae0ed52
to
c308c96
Compare
Split read and write functions, using function pointers do not work as the parameters has deviating qualifiers (const vs non-const).
c308c96
to
76bc4d1
Compare
I would welcome review on this one. This PR together with #3344 are the last fixes to prepare GRASS code to work with and comply to C23 standard. |
I've read about this C prototype discussion when Clang implemented it, and I'm favorable for it. From your changes, I could find one that I fully understood (the one that added a void instead of nothing in the prototype). I understand that it could only be breaking for users that would use our software as a library, and would have been using it potentially inappropriately. Including in 8.4 is appropriate then. I also understand the better warnings that the compiler can emit, so it's a bigger win. As for the other lines, I'm not sure I understand the consequences of it, so I'm not sure how to review it confidently. |
It is not a Clang-thing, but a C23 standard feature (for background see eg. https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2841.htm). It is a step to modernise the code, enable it to build with C23+ compilers. We now require C11 conformance, but that may change, and this PR doesn't break anything for C11. The more immediate cause for this PR is better described in #2891. These are the compiler warnings issued:
If you don't feel comfortable with reviewing it, I won't blame you for not to :-). But studying the "old" code is quite educational. Pretty clever use of function pointers. |
I'm ok with the change, and I tested with clang 14 on FreeBSD 14, and it's OK. |
@lbartoletti Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was confused for a minute whether there is some new function prototype thing, but the sentence from the linked document clarified it for me:
Submission Date: 2021-10-10
...
Function declarators without prototypes have been an obsolescent feature since the very first ISO C standard. 33 years of advance warning should be enough.
:-)))
And FYI, gcc14 will raise error on old C code: https://gcc.gnu.org/gcc-14/porting_to.html#c |
…#2901) Split read and write functions, using function pointers do not work as the parameters has deviating qualifiers (const vs non-const). It does introduce some repetitive code, but do not remove the correctly used const qualifiers in the Rast3d_key_set_double() and similar functions. Note: the "write" part of Rast3d_readWriteWindow() in windowio.c is never used and therefore removed. A write function is easily implemented if needed.
Split read and write functions, using function pointers do not work as the parameters has deviating qualifiers (const vs non-const).
This is a, perhaps preferable, alternative to #2891.
It does introduce some repetitive code, but do not remove the correctly used const qualifiers in the
Rast3d_key_set_double()
and similar functions.Note: the "write" part of
Rast3d_readWriteWindow()
inwindowio.c
is never used and therefore removed. A write function is easily implemented if needed.