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

r.viewshed: initialize struct member before using struct #4232

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

ymdatta
Copy link
Contributor

@ymdatta ymdatta commented Aug 26, 2024

This was reported by cppcheck tool. Technically, we are not using the member in the struct variable in any of the subsequent functions, but it's a good practice to initialize all the struct members whenever possible.

Initial errors from cppcheck tool:

image

After applying the patch:

image

Additional information:

  1. Machine used: Virtual Machine running Ubuntu 22.04.4 LTS.
  2. Reproduction rate: 100%, reproducible every time.
  3. Architecture: Not specific to any underlying architecture.

Testing done:

Ran the complete test_r_viewshed.py gunittest from the testsuite and it was successful.

This was reported by cppcheck tool. Technically, we are not using
the member in the struct variable in any of the subsequent
functions, but it's a good practice to initialize all the struct
members whenever possible.

Signed-off-by: Mohan Yelugoti <[email protected]>
@github-actions github-actions bot added raster Related to raster data processing C++ Related code is in C++ module labels Aug 26, 2024
@neteler neteler added this to the 8.5.0 milestone Aug 27, 2024
@echoix
Copy link
Member

echoix commented Sep 11, 2024

What's the reasoning for using -1 as the initial value for angle? (I understand that 0 is a valid value that might cause confusion)

@ymdatta
Copy link
Contributor Author

ymdatta commented Sep 12, 2024

I wanted a value that's different from 0 and in the same module, I saw usage of -1 in two places for initialization: grass.cpp:L242 and grass.cpp:L460. So, I went ahead with -1.

@echoix
Copy link
Member

echoix commented Sep 12, 2024

typedef struct event_ {
dimensionType row, col; // location of the center of cell
// 3 elevation values:
// elev[0]: entering
// elev[1]: center
// elev[2]: exiting
surface_type elev[3]; // elevation here
double angle;
char eventType;
// type of the event: ENTERING_EVENT, EXITING_EVENT, CENTER_EVENT
} AEvent;

I see here that the angle is a double. Should the suggestion of using -1.0 (like using 0.0 instead of 0) be applied here too, even if you pointed out occurrences where -1 was used (without the dot zero)?

@ymdatta
Copy link
Contributor Author

ymdatta commented Sep 13, 2024

I agree with you, I think -1.0 is the right literal to be used here. I have made the changes. Thanks!

@echoix echoix merged commit c840f3c into OSGeo:main Sep 16, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ Related code is in C++ module raster Related to raster data processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants