Skip to content

src: fix unchecked return warning from coverity #42176

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

Closed
mhdawson wants to merge 2 commits into from
Closed

src: fix unchecked return warning from coverity #42176

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2142142
src: fix unchecked return warning from coverity
mhdawson Mar 1, 2022
47371d0
squash: add CHECK_EQ to other _init calls
mhdawson Mar 2, 2022
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
4 changes: 2 additions & 2 deletions src/env.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ void Environment::InitializeLibuv() {
uv_prepare_init(event_loop(), &idle_prepare_handle_);
uv_check_init(event_loop(), &idle_check_handle_);

uv_async_init(
CHECK_EQ(0, uv_async_init(

@addaleax addaleax Mar 2, 2022

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.

There are a large number of _init calls in this function, isn’t it odd to single some out?

@mhdawson mhdawson Mar 2, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@addaleax it's the only one that coverity reported as a problem. Are you suggesting I should add them to the others as as well or not add at all?

@mhdawson mhdawson Mar 2, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The difference might be that we check the return from uv_check_init everywhere else. This is the actual report:

CID 239987 (#1 of 1): Unchecked return value (CHECKED_RETURN)
3. check_return: Calling uv_async_init without checking return value (as is done elsewhere 15 out of 16 times).

@addaleax addaleax Mar 2, 2022

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.

I see, that makes sense – Ideally, we’d check them everywhere, yes.

@mhdawson mhdawson Mar 2, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The _init methods don't seem to use in other places. I'll add to the other _init methods as well

@mhdawson mhdawson Mar 2, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@addaleax added to the other methods in this function.

event_loop(),
&task_queues_async_,
[](uv_async_t* async) {
Expand All @@ -568,7 +568,7 @@ void Environment::InitializeLibuv() {
HandleScope handle_scope(env->isolate());
Context::Scope context_scope(env->context());
env->RunAndClearNativeImmediates();
});
}));
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_prepare_handle_));
uv_unref(reinterpret_cast<uv_handle_t*>(&idle_check_handle_));
uv_unref(reinterpret_cast<uv_handle_t*>(&task_queues_async_));
Expand Down