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

fixed many COPY_INSTEAD_OF_MOVE Coverity warnings #5944

Merged
merged 6 commits into from
Feb 5, 2024

Conversation

firewave
Copy link
Collaborator

@firewave firewave commented Feb 4, 2024

No description provided.

@@ -194,7 +194,7 @@ int CppCheckExecutor::check(int argc, const char* const argv[])
mStdLogger = new StdLogger(settings);

CppCheck cppCheck(*mStdLogger, true, executeCommand);
cppCheck.settings() = settings;
cppCheck.settings() = std::move(settings);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a bogus warning. The object is referenced in StdLogger.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The Settings object should also be const after construction to make sure it is no longer modified afterwards. That is still WIP.

Also the copy will go away after #4964 is done.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@firewave
Copy link
Collaborator Author

firewave commented Feb 5, 2024

The accessMoved false positive is already tracked in https://trac.cppcheck.net/ticket/12174.

@firewave firewave marked this pull request as ready for review February 5, 2024 01:27
@firewave firewave marked this pull request as draft February 5, 2024 01:30
@firewave firewave marked this pull request as ready for review February 5, 2024 02:00
Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

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

seems like we could easily find lots of more performance issues..

@firewave
Copy link
Collaborator Author

firewave commented Feb 5, 2024

seems like we could easily find lots of more performance issues..

I gave it a short spin in the profiler and it doesn't have much of an impact. Guess the most important were already found earlier by review and profiling.

@firewave firewave merged commit a2ecf17 into danmar:main Feb 5, 2024
64 checks passed
@firewave firewave deleted the coverity-move branch February 5, 2024 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants