Skip to content
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

Jump to bottom

src: fix unchecked return warning from coverity #42176

Closed
wants to merge 2 commits into from
Closed

src: fix unchecked return warning from coverity #42176

Changes from all commits
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/env.cc
View file
Open in desktop
Expand Up @@ -545,21 +545,21 @@ void Environment::InitializeLibuv() {
CHECK_EQ(0, uv_timer_init(event_loop(), timer_handle()));
uv_unref(reinterpret_cast<uv_handle_t*>(timer_handle()));

uv_check_init(event_loop(), immediate_check_handle());
CHECK_EQ(0, uv_check_init(event_loop(), immediate_check_handle()));
uv_unref(reinterpret_cast<uv_handle_t*>(immediate_check_handle()));

uv_idle_init(event_loop(), immediate_idle_handle());
CHECK_EQ(0, uv_idle_init(event_loop(), immediate_idle_handle()));

uv_check_start(immediate_check_handle(), CheckImmediate);
CHECK_EQ(0, uv_check_start(immediate_check_handle(), CheckImmediate));

// Inform V8's CPU profiler when we're idle. The profiler is sampling-based
// but not all samples are created equal; mark the wall clock time spent in
// epoll_wait() and friends so profiling tools can filter it out. The samples
// still end up in v8.log but with state=IDLE rather than state=EXTERNAL.
uv_prepare_init(event_loop(), &idle_prepare_handle_);
uv_check_init(event_loop(), &idle_check_handle_);
CHECK_EQ(0, uv_prepare_init(event_loop(), &idle_prepare_handle_));
CHECK_EQ(0, uv_check_init(event_loop(), &idle_check_handle_));

uv_async_init(
CHECK_EQ(0, uv_async_init(
Copy link
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?

Copy link
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?

Copy link
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).

Copy link
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.

Copy link
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

Copy link
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