-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
permission: address coverity warning #50215
Conversation
Review requested:
|
Coverity has a new set of warnings. Seeing what we think about fixing those by starting with this one. If there is a possitive response I'd plan to fix in larget baches. The warning was: 212 if (is_wildcard_node || is_last_char) {
213 std::string node_path = path.substr(parent_node_prefix_len, i);
CID 329770 (#1 of 1): COPY_INSTEAD_OF_MOVE (COPY_INSTEAD_OF_MOVE)
1. copy_constructor_call: node_path is passed-by-value as parameter to CreateChild when it could be moved instead.
Use std::move(node_path) instead of node_path.
214 current_node = current_node->CreateChild(node_path); |
Which compiler are you using? I didn't get these warnings building on macOS
|
@RafaelGSS they are not from the compiler from but our Coverity static analysis - https://scan.coverity.com/projects/node-js |
src/permission/fs_permission.cc
Outdated
@@ -211,7 +211,7 @@ void FSPermission::RadixTree::Insert(const std::string& path) { | |||
|
|||
if (is_wildcard_node || is_last_char) { | |||
std::string node_path = path.substr(parent_node_prefix_len, i); | |||
current_node = current_node->CreateChild(node_path); | |||
current_node = current_node->CreateChild(std::move(node_path)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't really change anything (except maybe shut up coverity) because CreateChild takes a value, not a reference. What you want instead is this:
diff --git a/src/permission/fs_permission.h b/src/permission/fs_permission.h
index 244e95727a..80374a08c7 100644
--- a/src/permission/fs_permission.h
+++ b/src/permission/fs_permission.h
@@ -31,7 +31,7 @@ class FSPermission final : public PermissionBase {
Node() : wildcard_child(nullptr), is_leaf(false) {}
- Node* CreateChild(std::string prefix) {
+ Node* CreateChild(const std::string& prefix) {
if (prefix.empty() && !is_leaf) {
is_leaf = true;
return this;
Or Node* CreateChild(std::string&& prefix) {
+ std::move(...)
. Look at other call sites to determine which is most appropriate.
@bnoordhuis thanks, updated |
@mhdawson can you rebase? |
The latest version of coverity has suggestions on how to improve formance. Address one of these suggestions. Signed-off-by: Michael Dawson <midawson@redhat.com>
@RafaelGSS rebased on squashed commits |
The latest version of coverity has suggestions on how to improve formance. Address one of these suggestions. Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: #50215 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Landed in 3c1b4b3 |
The latest version of coverity has suggestions on how to improve formance. Address one of these suggestions. Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: nodejs#50215 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
The latest version of coverity has suggestions on how to improve formance. Address one of these suggestions. Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: #50215 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
The latest version of coverity has suggestions on how to improve formance. Address one of these suggestions. Signed-off-by: Michael Dawson <midawson@redhat.com> PR-URL: #50215 Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
The latest version of coverity has suggestions on how to improve formance. Address one of these suggestions.