worker: add missing initialization#41421
Conversation
Add missing initialization reported by coverity Signed-off-by: Michael Dawson <mdawson@devrus.com>
nodejs-github-bot
added
c++
There was a problem hiding this comment.
uv_thread_t is an opaque type, we should not rely on the fact that 0 may currently be a valid initailizer for it.
Usage of tid_ is restricted to the condition that thread_joined_ is false, and conversely thread_joined_ is only set to false when tid_ is initialized. Consequently, this seems to be a false positive.
Edit: To be clear: This also reduces readability because initializing tid_ makes it looks like the variable can always be accessed, even when the thread is not running, which is not valid usage of it. However, I think we’re using C++17 now, so maybe we could drop thread_joined_ and instead make tid_ an std::optional<uv_thread_t>?
|
PR with that suggestion: #41453 |
|
Ok, I'll check the coverity report after the change proposed in #41453 and if it still reports an issue we can flag it as a false positive. |
Refs: nodejs#41421 PR-URL: nodejs#41453 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
|
#41453 seems to have resolve the warning, closing. |
Refs: nodejs#41421 PR-URL: nodejs#41453 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
Refs: nodejs#41421 PR-URL: nodejs#41453 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>
5 participants
Add missing initialization reported by coverity
Signed-off-by: Michael Dawson mdawson@devrus.com