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.

Sign up for GitHub

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

Jump to bottom

[GeoMechanicsApplication] Fixed a few issues found by Coverity #12713

Merged
merged 4 commits into from
Oct 1, 2024
Merged

[GeoMechanicsApplication] Fixed a few issues found by Coverity #12713

merged 4 commits into from
Oct 1, 2024
+84 −96

Conversation

avdg81
Copy link
Contributor

@avdg81 avdg81 commented Oct 1, 2024 •
edited
Loading

📝 Description

This PR fixes a few issues that have been found by Coverity.

🆕 Changelog

Changes include:

  • Fixed a memory leak in a C++ unit test (which, however, is not being run at the moment).
  • Removed some structurally and logically dead code.
  • "Unnamed" a few unused function parameters.
  • Initialized some pointer members to nullptr.
  • Renamed a few data members to comply with the Kratos Style Guide.

@avdg81
Fixed a memory leak in the test code
3f2df88 
Also renamed a local variable to comply with the Kratos Style Guide.
@avdg81
Removed some structurally dead code
bf6bd30 
Also "unnamed" a few unused function parameters.
@avdg81
Initialize a few pointer members to nullptr
a44ad57 
Also renamed these data member to comply with the Kratos Style Guide (prepended the names with `m`).
@avdg81
Removed some logically dead code
e496954
@avdg81 avdg81 added the GeoMechanics Issues related to the GeoMechanicsApplication label Oct 1, 2024
@avdg81 avdg81 self-assigned this Oct 1, 2024
@avdg81 avdg81 requested review from WPK4FEM and rfaasse October 1, 2024 12:05
rfaasse
rfaasse approved these changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

Thanks for fixing these, seems coverity already showed some value 👍

WPK4FEM
WPK4FEM approved these changes Oct 1, 2024
View reviewed changes
Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

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

I see the removal of dead else branch, replacing p --> mp and the pointer initializations with nullptr. That looks o.k. to me.

applications/GeoMechanicsApplication/python_scripts/piping_analysis.py
@@ -115,30 +115,23 @@ def RunSolutionLoop(self):

KratosMultiphysics.Logger.PrintInfo(self._GetSimulationName(), "--------------------------------------", " ")

is_piping = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed this was creation of an unreachable else branch

@avdg81 avdg81 merged commit 1289be4 into master Oct 1, 2024
9 of 11 checks passed
@avdg81 avdg81 deleted the geo/fix-some-issues-found-by-coverity branch October 1, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rfaasse rfaasse rfaasse approved these changes

@WPK4FEM WPK4FEM WPK4FEM approved these changes

Assignees

@avdg81 avdg81

Labels
GeoMechanics Issues related to the GeoMechanicsApplication
Projects
GeoMechanicsApplication (Deltares)
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@avdg81 @rfaasse @WPK4FEM