Skip to content

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

Merged
yosefe merged 1 commit into
openucx:masterfrom
alinask:topic/ucp-wireup-cm-cov
Jan 7, 2021
Merged

yosefe merged 1 commit into
openucx:masterfrom
alinask:topic/ucp-wireup-cm-cov

Conversation

@alinask

@alinask alinask commented Jan 5, 2021

Copy link
Copy Markdown
Contributor

No description provided.

@alinask
alinask force-pushed the topic/ucp-wireup-cm-cov branch from bc5a70f to d45a32e Compare January 5, 2021 19:55
@alinask alinask changed the title UCP/UCT/TEST/EXAMPLES - Fix several coverity issues. UCP/UCT/TEST/EXAMPLES: Fix several coverity issues. Jan 5, 2021
@alinask

alinask commented Jan 6, 2021

Copy link
Copy Markdown
Contributor Author

@yosefe please review

if (err_handling_opt.failure_mode == FAILURE_MODE_SEND) {
fprintf(stderr, "Emulating unexpected failure on server side, client "
"should detect error by keepalive mechanism\n");
free(msg);

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

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?

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

Comment thread src/ucp/wireup/wireup_cm.c Outdated
if (status != UCS_OK) {
ucs_warn("failed to reject connect request %p on listener %p",
conn_request, listener);
ucs_warn("failed to reject connect request on listener %p", listener);

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 Coverity exception here? since ucs_warn does really not dereference the pointer, just prints it

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.

you mean to add a comment to make coverity ignore this?

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

test_sockaddr.cc - NEGATIVE_RETURNS - "index" is passed to a parameter
that cannot be negative. [Note: The source code implementation of the
function has been overridden by a builtin model.]

uct_cm.c - PRINTF_ARGS - Argument "self->iface" to format specifier "%p"
was expected to have pointer type but has type "struct uct_base_iface".

ucp_hello_world.c - USE_AFTER_FREE - Dereferencing freed pointer "msg"

wireup_cm.c - USE_AFTER_FREE - Passing freed pointer "conn_request" as
an argument to "ucs_log_dispatch" ("uct_listener_reject" frees
"conn_request".).
@alinask
alinask force-pushed the topic/ucp-wireup-cm-cov branch from d45a32e to a245659 Compare January 6, 2021 12:52

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

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

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.

@alinask

alinask commented Jan 7, 2021

Copy link
Copy Markdown
Contributor Author

@yosefe can you have another look please?

@yosefe
yosefe merged commit 885bd63 into openucx:master Jan 7, 2021
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.

3 participants