X Tutup
Skip to content

fixed many COPY_INSTEAD_OF_MOVE Coverity warnings#5944

Merged
firewave merged 6 commits intodanmar:mainfrom
firewave:coverity-move
Feb 5, 2024
Merged

fixed many COPY_INSTEAD_OF_MOVE Coverity warnings#5944
firewave merged 6 commits intodanmar:mainfrom
firewave:coverity-move

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

@firewave firewave commented Feb 4, 2024

No description provided.


CppCheck cppCheck(*mStdLogger, true, executeCommand);
cppCheck.settings() = settings;
cppCheck.settings() = std::move(settings);
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@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

X Tutup