Skip to content

UCP/UCT/TEST/EXAMPLES: Fix several coverity issues. #6094

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

Merged
yosefe merged 1 commit into from
Jan 7, 2021
Merged

UCP/UCT/TEST/EXAMPLES: Fix several coverity issues. #6094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions examples/ucp_hello_world.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ static int run_ucx_server(ucp_worker_h ucp_worker)
"should detect error by keepalive mechanism\n");
free(msg);

@yosefe yosefe Jan 6, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we add exit() after raise() instead, since exit() has noreturn attribute

@alinask alinask Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

with exit(1) to indicate an error happened?

@yosefe yosefe Jan 6, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes

raise(SIGKILL);
exit(1);
}

peer_addr_len = msg->data_len;
Expand Down
1 change: 1 addition & 0 deletions src/ucp/wireup/wireup_cm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,7 @@ void ucp_cm_server_conn_request_cb(uct_listener_h listener, void *arg,
err_reject:
status = uct_listener_reject(listener, conn_request);
if (status != UCS_OK) {
/* coverity[pass_freed_arg] */
ucs_warn("failed to reject connect request %p on listener %p",
conn_request, listener);
}
Expand Down
2 changes: 1 addition & 1 deletion src/uct/base/uct_cm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ UCS_CLASS_INIT_FUNC(uct_cm_t, uct_cm_ops_t* ops, uct_iface_ops_t* iface_ops,

return UCS_STATS_NODE_ALLOC(&self->iface.stats, &uct_cm_stats_class,
ucs_stats_get_root(), "%s-%p", "iface",
self->iface);
&self->iface);
}

UCS_CLASS_CLEANUP_FUNC(uct_cm_t)
Expand Down
2 changes: 1 addition & 1 deletion test/gtest/uct/ib/test_sockaddr.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1211,7 +1211,7 @@ class test_uct_cm_sockaddr_stress : public test_uct_cm_sockaddr {
int index;

index = get_ep_index(ep);
ASSERT_GE(index, 0);
ASSERT_TRUE(index >= 0);

@dmitrygx dmitrygx Jan 6, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do you see this warning since different signedness?
but if I'm not mistaken, they both have to be signed integer

@alinask alinask Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

later on in this function index is used to access a field in an array, so therefore it can't be a negative value.

EXPECT_LT(index, (2 * m_clients_num));

pthread_mutex_lock(&m_lock);
Expand Down