Skip to content
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

checks: Fix uninitialized, scope, leak, const issues in C #2250

Merged
merged 23 commits into from
Mar 17, 2022

Conversation

lbartoletti
Copy link
Contributor

Some fixes on various error/alert/performance issues spotted by clang and cppcheck (there are others, but I prefer to go step by step and know if it suits you ☺️ )

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. This is great. The const reference changes are clear, the free as well, the other (four) changes may need double checking.

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be safer for the two mode functions to return nan or inf in case the input is empty, but it is a misuse of the function, so the return value is sort of undefined in that case. You can consider mentioning this in a comment in the code there (lib/calc/xmode.c and xnmode.c).

@wenzeslaus wenzeslaus added C Related code is in C bug Something isn't working labels Mar 17, 2022
@wenzeslaus wenzeslaus added this to the 8.2.0 milestone Mar 17, 2022
@wenzeslaus wenzeslaus merged commit 2bb77ed into OSGeo:main Mar 17, 2022
@wenzeslaus
Copy link
Member

Thanks! Next time, when splitting the work, consider that the commit message summary (PR title) should be descriptive (although I managed to come up with hopefully descriptive enough line at the end; your individual commit messages were very helpful! ...and are included in the commit message body).

@lbartoletti
Copy link
Contributor Author

OK, thanks!

@lbartoletti lbartoletti deleted the various_fixes branch March 18, 2022 05:08
@wenzeslaus wenzeslaus changed the title Various fixes checks: Fix uninitialized, scope, leak, const issues in C Apr 27, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
* make_loc.c: reduce the scope of the variable 'path'
* make_loc.c: remove useless assignment of l_1 and l_2 since they are overwritten after
* make_loc.c: l_1 and l_2 are already checked before
* xmode.c: mode: initialize mode_v to avoid a false-positive warning
* xnmode.c: mode: initialize mode_v to avoid a false-positive warning
* gs3.c: Gs_update_attrange: initialize min and max to avoid a false-positive warning
* gs3.c: Gs_load_3dview: reduce scope of the variable 'pt'
* gs3.c: Gs_load_3dview: const geosurf
* mm_utils: MEMORY_LOG: pass str by reference
* dataquad.c: quad_data_new: fix memory leak data
* dataquad.c: quad_compare: avoid use of null data
* dataquad.c: quad_add_data: reduce scope of variables
* cr_from_a.c: main: fix resource leak: fp
* i.atcorr: various const ref performance
* bitmap.c: fix memory leak CWE: 401
* bitmap/sparse.c: fix memory leak CWE: 401
* sparse.c: BM_set_sparse: Tcount is never used
* sparse.c: BM_file_write_sparse: reduce scope of cnt
* r.terraflow/sweep.h: performs initialization in initialization list
* add missing const, various const ref performance

Issues detected by clang and cppcheck.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
* make_loc.c: reduce the scope of the variable 'path'
* make_loc.c: remove useless assignment of l_1 and l_2 since they are overwritten after
* make_loc.c: l_1 and l_2 are already checked before
* xmode.c: mode: initialize mode_v to avoid a false-positive warning
* xnmode.c: mode: initialize mode_v to avoid a false-positive warning
* gs3.c: Gs_update_attrange: initialize min and max to avoid a false-positive warning
* gs3.c: Gs_load_3dview: reduce scope of the variable 'pt'
* gs3.c: Gs_load_3dview: const geosurf
* mm_utils: MEMORY_LOG: pass str by reference
* dataquad.c: quad_data_new: fix memory leak data
* dataquad.c: quad_compare: avoid use of null data
* dataquad.c: quad_add_data: reduce scope of variables
* cr_from_a.c: main: fix resource leak: fp
* i.atcorr: various const ref performance
* bitmap.c: fix memory leak CWE: 401
* bitmap/sparse.c: fix memory leak CWE: 401
* sparse.c: BM_set_sparse: Tcount is never used
* sparse.c: BM_file_write_sparse: reduce scope of cnt
* r.terraflow/sweep.h: performs initialization in initialization list
* add missing const, various const ref performance

Issues detected by clang and cppcheck.
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
* make_loc.c: reduce the scope of the variable 'path'
* make_loc.c: remove useless assignment of l_1 and l_2 since they are overwritten after
* make_loc.c: l_1 and l_2 are already checked before
* xmode.c: mode: initialize mode_v to avoid a false-positive warning
* xnmode.c: mode: initialize mode_v to avoid a false-positive warning
* gs3.c: Gs_update_attrange: initialize min and max to avoid a false-positive warning
* gs3.c: Gs_load_3dview: reduce scope of the variable 'pt'
* gs3.c: Gs_load_3dview: const geosurf
* mm_utils: MEMORY_LOG: pass str by reference
* dataquad.c: quad_data_new: fix memory leak data
* dataquad.c: quad_compare: avoid use of null data
* dataquad.c: quad_add_data: reduce scope of variables
* cr_from_a.c: main: fix resource leak: fp
* i.atcorr: various const ref performance
* bitmap.c: fix memory leak CWE: 401
* bitmap/sparse.c: fix memory leak CWE: 401
* sparse.c: BM_set_sparse: Tcount is never used
* sparse.c: BM_file_write_sparse: reduce scope of cnt
* r.terraflow/sweep.h: performs initialization in initialization list
* add missing const, various const ref performance

Issues detected by clang and cppcheck.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working C Related code is in C
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants